Skip to content

Run data managers#30

Merged
afgane merged 9 commits intomasterfrom
dm
Apr 5, 2017
Merged

Run data managers#30
afgane merged 9 commits intomasterfrom
dm

Conversation

@bgruening
Copy link
Copy Markdown
Member

This scripts let you define some data managers with different items and parameters to populate Galaxy instances like Galaxy flavors.

@bgruening bgruening requested a review from afgane April 5, 2017 12:34
@bgruening
Copy link
Copy Markdown
Member Author

@afgane do you also have the rights to upload a new version to pypi?

@bgruening
Copy link
Copy Markdown
Member Author

ping @bebatut
This might be interesting for Aisam as well.

@jmchilton
Copy link
Copy Markdown
Member

@bgruening I think I only have rights to upload to pypi currently - if you get the HISTORY.rst file in shape I'll create a new version.

@bgruening
Copy link
Copy Markdown
Member Author

@jmchilton I added it here: 980a274

The HISTORY.rst version should remain with .dev0 - the deployment scripts automate removing that. Also bumping the version since there are some new features and scripts.
@jmchilton
Copy link
Copy Markdown
Member

Thanks @bgruening - I tweaked that a bit. When this gets merged I will release this.

@bgruening
Copy link
Copy Markdown
Member Author

Thanks John, waiting for @afgane or @mvdbeek and then we are ready to go.

Comment thread ephemeris/run_data_managers.py Outdated
gi = GalaxyInstance(url=url, email=args.user, password=args.password)

# should test valid connection
log.info("List of valid histories: %s" % gi.histories.get_histories())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe gi.users.get_current_user() is a little more useful here?

Comment thread ephemeris/run_data_managers.py Outdated
for data_table in dm.get('data_table_reload', []):
# reload two times
for i in range(2):
gi.make_get_request(urlparse.urljoin(url, 'api/tool_data/%s/reload' % data_table))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is only going to work if either an external message queue is running, or galaxy is running in run.sh mode.
Our current standard setup can't send the reload task from uwsgi to the job handlers.
So if you for example add a new genome with a genome data manager, and then try to create a bowtie2 index for that genome there is a good chance this will fail because the handler doesn't know about the new genome.
I have added automatic reloading of changed data tables (without an explicit API call) in galaxyproject/galaxy#3533, but right now we still need to have watchdog installed for this to work (we should make a wheel for that).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe I was lucky but this worked for me several time in the Galaxy-Docker setup and the external message queue is not properly configured.
We could add a note to the script that watchdog needs to be installed? Would this be ok?

@mvdbeek
Copy link
Copy Markdown
Member

mvdbeek commented Apr 5, 2017

Apart from my 2 comments (the second one is definitely not blocking a merge) this looks good, thanks @bgruening.

Copy link
Copy Markdown
Contributor

@afgane afgane left a comment

Choose a reason for hiding this comment

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

Looks good; just a couple of minor inline comments.

Thanks!

Comment thread ephemeris/run_data_managers.py Outdated
description='Running Galaxy data managers in a defined order with defined parameters.')
parser.add_argument("-v", "--verbose", help="Increase output verbosity.",
action="store_true")
parser.add_argument("--config", required=True, help="Path the YAML config file.")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should probably be a bit more explicit to say [Required] Path to the YAML config file with the list of data managers and data to install. There's also a missing to in the current help text.

Sorry to be a stickler on documentation but if a new person comes along and wants to use this, the docs is the first thing they'll see and based on that make a decision if something is usable or not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sure, I added your sentence. It was required before so I left that as it is. argparse will give indication that this parameter is required.

Comment thread ephemeris/run_data_managers.py Outdated
for data_table in dm.get('data_table_reload', []):
# reload two times
for i in range(2):
gi.make_get_request(urlparse.urljoin(url, 'api/tool_data/%s/reload' % data_table))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not going to work with the current imports. Need to just import urlparse vs. from urlparse import urlparse.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You mean the python2 case? Let me test this and fix it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed

Copy link
Copy Markdown
Contributor

@afgane afgane Apr 5, 2017

Choose a reason for hiding this comment

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

Hmm, actually, it's not going to work on py3 either because urljoin is not a module within urllib.parse.urlparse. It may be best to import urljoin directly for both py2 & 3 and then change this line to just use it:

try:
  from urllib.parse import urljoin
except ImportError:
  from urlparse import urljoin

@afgane afgane merged commit 5a2c696 into master Apr 5, 2017
@afgane
Copy link
Copy Markdown
Contributor

afgane commented Apr 5, 2017

Thanks for all the edits @bgruening.

@jmchilton
Copy link
Copy Markdown
Member

Version 0.5.0 of ephemeris has been published - https://pypi.python.org/pypi/ephemeris.

@bgruening bgruening deleted the dm branch April 6, 2017 13:31
@bgruening
Copy link
Copy Markdown
Member Author

@jmchilton thanks - unfortunately I think we need #32 as well :(

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.

4 participants