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 command to download artifacts from Jenkins #12

Merged
merged 11 commits into from
Sep 3, 2013

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Aug 30, 2013

This PR adds a new download command to omego to download artifacts from the CI server which should be used as follows

omego download -h
omego download server
omego download server --skipunzip true
omego download win --branch OMERO-merge-stable

The major changes in this PR are the following

  • the artifact logic (class and command) is moved to an artifacts.py (since the download command did not fit with upgrade.py)
  • corresponding tests are added and built as part of the Travis build
  • Artifacts.download() does not fail if --skipunzip true is false. The Stop() is moved to the UpgradeCommand instead

More generally, should we keep maintaining our own Artifacts class or start using a 3d party library e.g. jenkinsapi which already has all this logic in place including MD5 checksumming etc... /cc @manics, @joshmoore

choices=Artifacts.get_artifacts_list().keys(),
help="The artifact to download from the CI server")

Add = EnvDefault.add
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a common location for the OMERO-qa-upgrade.py-like parameters? i.e. can there be any refactoring to prevent repetition?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed.
One option would be to use the same approach as scc and define a ArtifactsCommand superclass.
Else we could create a JenkinsEnv class and call JenkinsEnv.add(self.parser).
Any preference? alternate option?

Copy link
Member

Choose a reason for hiding this comment

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

In general, we will eventually want smarter abstract classes, but JenkinsEnv especially if we decide to refactor jobs, use jenkinsapi, etc. might be a good idea.

@sbesson
Copy link
Member Author

sbesson commented Sep 2, 2013

Last set of commits refactor the Jenkins options under a single wrapper parser class.
Only breaking change from previous behaviour is that the --skipunzip option is now turned into a flag rather than a parameter/value pair.

@manics
Copy link
Member

manics commented Sep 3, 2013

Works fine, good to merge.

joshmoore added a commit that referenced this pull request Sep 3, 2013
Add command to download artifacts from Jenkins
@joshmoore joshmoore merged commit 8b2c336 into ome:master Sep 3, 2013
@sbesson sbesson deleted the artifacts branch September 3, 2013 13:29
@sbesson sbesson added this to the 0.1.1 milestone Dec 11, 2017
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.

3 participants