Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix autoupdate for workflows with overlapping tool updates #1452

Merged

Conversation

bernt-matthias
Copy link
Contributor

An attempt to solve the problem that the workflow autoupdate does not update all workflows in a dir.
I think the problem occurs if the tool set of the workflows overlaps (e.g. galaxyproject/iwc#422).

The symptom that is fixed by my change is that config.updated_repos gets empty for some workflows which are therefore skipped.

I guess a cleaner way to fix this might be to uninstall all tools again after a workflow was processed?

@bernt-matthias bernt-matthias force-pushed the topic/update-all-workflows branch from 3906a83 to e49b080 Compare June 2, 2024 15:39
@mvdbeek
Copy link
Member

mvdbeek commented Jun 10, 2024

I would really appreciate it if we didn't need one instance per workflow.
To make it simpler, we could update all workflows by dropping if config.updated_repos.get(workflow.path) or kwds.get("engine") == "external_galaxy":.

We then check if any tool has actually been upgraded. That means it will also work for local non-TS tools (e.g. built-in tools and converters). The cost of running the refactoring action is minimal, compared to one instance per workflow.
This way we also get syntax upgrades or fixes "for free" if we decide to do that in the future

@bernt-matthias
Copy link
Contributor Author

We then check if any tool has actually been upgraded.

Would we need to implement this? Or is this already included?

Copy link
Member

@mvdbeek mvdbeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @bernt-matthias, added a test for this. Let's merge if it passes

@mvdbeek mvdbeek merged commit 3972552 into galaxyproject:master Sep 24, 2024
12 of 14 checks passed
@mvdbeek mvdbeek changed the title Autoupdate: start a separate instance for each workflow Fix autoupdate for workflows with overlapping tool updates Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants