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

Implement recursive shed uploads #68

Merged
merged 3 commits into from Feb 12, 2015

Conversation

Projects
None yet
3 participants
@erasche
Copy link
Member

erasche commented Feb 12, 2015

I was about to write a small wrapper to upload dozens of tools to a shed using planemo, but then I realised I shouldn't write something new, it should be added to planemo. So, I've added a --recurse option which lets you launch planemo as

$ planemo shed_upload --recurse --shed_target testtoolshed path/to/bgruenings/billions/of/tools/ 

and have it push all of the tools at once, rather than with messy for loops on the command line. Anything with a .shed.yml in it will be attempted to be uploaded. I didn't know (and didn't bother to check) how the caller used the return code, so I settled for "if everything returns OK, then we return OK. If one thing returns a -1, we return a -1"

Eric Rasche
@bgruening

This comment has been minimized.

Copy link
Member

bgruening commented Feb 12, 2015

Does it makes sense to check before uploading if anything has changed?

@erasche I have noticed the path in your example ;)

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Feb 12, 2015

Looks good overall - make lint is fairly picky and doesn't like it though - I can fix that post merge if you would like.

Also - I think we should allow -r as an alias. Objections?

@erasche

This comment has been minimized.

Copy link
Member Author

erasche commented Feb 12, 2015

@bgruening I'm not aware of how one would accomplish that, since we're uploading .tar.gzs that get modified upon upload with replaced paths...it's just so easy to upload and wait for it to fail.

@jmchilton I'll fix linting issues, sorry about, my editor does pep8 automatically but I must've missed those. Definitely needs a -r alias. Will add in a minute.

Eric Rasche added some commits Feb 12, 2015

@erasche

This comment has been minimized.

Copy link
Member Author

erasche commented Feb 12, 2015

@jmchilton okay, passes flake and has a -r now.

jmchilton added a commit that referenced this pull request Feb 12, 2015

Merge pull request #68 from erasche/master
Implement recursive shed uploads

@jmchilton jmchilton merged commit 23c8e1d into galaxyproject:master Feb 12, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Feb 12, 2015

Awesome, thanks.

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Feb 12, 2015

Any clue if it will update stuff we don't want it to? Like lets say we have a tool_dependencies.xml on repository A in git that depends on a package X and obviously doesn't include a changeset - but in the toolshed A does have that changeset populated for package X in A's tool_dependencies.xml fie. If X is updated to a new changeset but not to a new installable revision - we don't want to update all of its dependencies right?

But if go an upload our unchanged tool_dependencies.xml from A - is it going to detect a change when it populates the new changeset (that would be annoying)? And if it does will it create a new installable revision (that would be pretty bad)?

Ping @blankenberg @InitHello

@erasche

This comment has been minimized.

Copy link
Member Author

erasche commented Feb 12, 2015

updated to a new changeset but not to a new installable revision

Ah, this is a toolshed side question. sorry, that took a while to understand.

back on the planemo end, there are some "bad" scenarios here, e.g.

  • tool1 depends on package_A, tool1 is uploaded before package_A due to search order. This means possibly multiple runs with -r will have to be done in order to ensure you're up to date.
  • ...

It's not a clean option, there's probably room for things like dependency graph building (i.e. ensure all children are pushed before parents) but the scope for that is a lot bigger than a simple -r option like I wanted. Definitely needs to be a TODO.

update stuff we don't want it to?

-r is a huge hammer for a possibly small nail; you told it you wanted everything pushed, so it's doing exactly that.

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Feb 12, 2015

Yeah definitely a tool shed side question - I think the client is right. I am a little concerned about the approach of pushing the whole repository at once - I feel like there might need to be a check that there is a diff we actually care about before pushing (kind of coming back to @bgruening's comment). I should do some experiments to convince myself one way or the other - but I guess your jenkins setup will figure that out for me :).

@erasche

This comment has been minimized.

Copy link
Member Author

erasche commented Feb 13, 2015

I think you have a valid concern there, but my experiments convinced me that it works "well enough"; unchanged repositories don't seem to be re-uploaded, and compared with the expense of trying to figure out of the files are truly unchanged (bearing in mind toolshed side modification of the files...) when the toolshed neatly rejects repeated uploads.

Indeed, haha, my jenkins set up with clobber absolutely everything in the TTS, so we'll see how that works for everyone and can iterate from that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment