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

Add autoupdate command #1065

Merged
merged 42 commits into from
Mar 17, 2021
Merged

Conversation

simonbray
Copy link
Member

A command for updating the requirements in XML tool files by checking for newer conda versions. Thanks to @lorrainealisha75 for working on this.

ping @mvdbeek, in this version a @GALAXY_VERSION@ token should no longer be necessary.

ping @wm75, I added a --dry-run flag as you suggested.

from planemo.config import planemo_option


def dry_run_option():
Copy link
Member Author

Choose a reason for hiding this comment

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

not sure if this should be moved to options.py or not

Copy link
Member

Choose a reason for hiding this comment

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

It is totally fine here, especially if only one command is using it.

def get_latest_versions(version_dict):
"""
Update a dict with current conda versions for tool requirements
"""
Copy link
Member

Choose a reason for hiding this comment

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

This is pretty awesome!

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you @jmchilton - I think this is ready for a review

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I take that back, there are still some things I can fix. Though any overall comments about the implementation would be welcome.

@bgruening
Copy link
Member

@simonbray some updated to the docs would be great. We can also use this in the IUC discussion then. Thanks.

@simonbray
Copy link
Member Author

@bgruening where should these be added? It seems the docs are autogenerated from the code.

@simonbray simonbray changed the title [WIP] Add autoupdate command Add autoupdate command Aug 13, 2020
@bgruening
Copy link
Member

@simonbray a similar page to this one maybe? https://github.com/galaxyproject/planemo/blob/master/docs/publishing.rst

We can also wait until we have a CI setup ready and then add documentation for a potential CI setup as well?

@simonbray simonbray changed the title Add autoupdate command [WIP] Add autoupdate command Aug 14, 2020
@simonbray
Copy link
Member Author

simonbray commented Aug 14, 2020

I ran it on the tools-iuc repository to test it out, have a look here: galaxyproject/tools-iuc@master...simonbray:autoupdate

It looks mostly okay, but there are a few bugs (e.g. galaxyproject/tools-iuc@master...simonbray:autoupdate#diff-937fe802b3fe3877efcbd5e6a1f6e36eL2) which I'll take a look at.

@simonbray
Copy link
Member Author

Testing of latest version on tools-iuc repo: galaxyproject/tools-iuc@master...simonbray:autoupdate8

@simonbray
Copy link
Member Author

Random idea that I want to write down before I forget it: this command should be able to update workflows as well as tools.

@simonbray simonbray closed this Oct 20, 2020
@simonbray simonbray reopened this Oct 20, 2020
@bgruening
Copy link
Member

@jmchilton what do you think about this? In particular in regards to your recent "update workflow" works?

@simonbray it would be nice to have this in planemo soon so we can finally add this functionality into IUC and close the loop:

upstream-github-releaseauto-conda-bumpauto-biocontainerauto-Galaxy-toolauto-mulled-containerauto-tool-installation-from-TSauto-tool-testingauto-workflow-bumpauto-workflow-testing

@jmchilton
Copy link
Member

Seems like a very solid plan - it will be interesting to see what types of errors we can catch at each step. Let me know if there is anything I can do to help.

@simonbray
Copy link
Member Author

@simonbray it would be nice to have this in planemo soon so we can finally add this functionality into IUC and close the loop

I think this is probably quite close to merging - I will have another look at it.

@lorrainealisha75 has also been working on implementing a CI bot here: https://github.com/planemo-autoupdate/autoupdate/blob/main/.github/workflows/update.yml

Where is the code for workflow autoupdates, is this part of planemo or somewhere else? I had the idea this could be done using the same autoupdate subcommand (similar to run, test etc. working with both tools and workflows). But I suppose this isn't really necessary.

@bgruening
Copy link
Member

@simonbray please have a look at galaxyproject/galaxy#11032 and related PRs. There are new APIs that can upgrade workflows and can upgrade tools, report on changed params or dropped connections etc ...

@simonbray
Copy link
Member Author

Nice! So if autoupdate got run on a workflow it could simply load it onto the chosen Galaxy (either local or external) and call this refactor API.

@bgruening
Copy link
Member

That is my understanding of the jmchilton mgic :)

``autoupdate`` assumes that XML tools are formatted in a certain way - in accordance with the `IUC best practices`_

<https://planemo.readthedocs.io/en/latest/standards/docs/best_practices/tool_xml.html/>`__. In particular:
``autoupdate`` assumes that XML tools are formatted in a certain way - in accordance with the `IUC best practices`_. In particular:

- the tool ``version`` attribute must be set to ``@TOOL_VERSION@+galaxy0`` or ``@TOOL_VERSION@+galaxy@GALAXY_VERSION``
Copy link

@davelopez davelopez Mar 16, 2021

Choose a reason for hiding this comment

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

Suggested change
- the tool ``version`` attribute must be set to ``@TOOL_VERSION@+galaxy0`` or ``@TOOL_VERSION@+galaxy@GALAXY_VERSION``
- the tool ``version`` attribute must be set to ``@TOOL_VERSION@+galaxy0`` or ``@TOOL_VERSION@+galaxy@VERSION_SUFFIX@``

;)

By the way, should this be @VERSION_SUFFIX@ instead of @GALAXY_VERSION@? The latest documentation says it should be @VERSION_SUFFIX@ but it seems a significant amount of tools use @GALAXY_VERSION@

Update: I answer myself after reading galaxy-iuc/standards#59 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes you are right - we decided on @VERSION_SUFFIX@ in the end but I didn't update here. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

It's @VERSION_SUFFIX@ , @GALAXY_VERSION@ was the whole +galaxyN .

@simonbray simonbray changed the title [WIP] Add autoupdate command Add autoupdate command Mar 16, 2021
@simonbray
Copy link
Member Author

Not sure about the failing test - it doesn't seem related and I can't reproduce it locally: https://travis-ci.org/github/galaxyproject/planemo/jobs/763104995

@simonbray
Copy link
Member Author

@jmchilton maybe you could have a look at this when you have time?

@jmchilton
Copy link
Member

I don't love that you're custom parsing XML outside the abstractions established in galaxy.tool_util.parser but you're also rewriting the XML explicitly so this wouldn't work with other future tool implementations, etc.. so I think this is ultimately totally fine in this case.

Really awesome work, thanks so much!

@jmchilton jmchilton merged commit 5516d12 into galaxyproject:master Mar 17, 2021
@simonbray
Copy link
Member Author

I don't love that you're custom parsing XML outside the abstractions established in galaxy.tool_util.parser

I definitely don't love it either but because of the need to rewrite the wrapper I'm not sure there's a good alternative.

Either way, thanks a lot for the merge!

@simonbray simonbray deleted the autoupdate-sb branch March 17, 2021 15:38
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.

6 participants