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

Allow adding new data tables without restart #4617

Merged
merged 10 commits into from Sep 19, 2017

Conversation

Projects
None yet
3 participants
@mvdbeek
Copy link
Member

commented Sep 14, 2017

This came up in galaxyproject/ephemeris#53:

We watch tool data tables now, which works fine for already installed data managers, but when installing new data managers these data tables are not yet being watched by the watchdog. This is addressed by 3f38e81.
Similarly, a tools DynamicOptions would (not very dynamically) hold on to the tool data tables that it knew about when it was first loaded. That is fixed by 01ddc99.
Finally there was a minor bug with caching of data manager tools leading to the guid not being properly set. This is fixed by 843d2e4

This PR also introduces an integration test case that installs 2 data managers from the toolshed and runs these. The second data manager needs the first data manager to successfully complete, so this exercises the problem we found in galaxyproject/ephemeris#53.

@mvdbeek

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2017

@galaxybot test this

@mvdbeek

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2017

Hmm, is there any way we can install watchdog for that integration test ?

@mvdbeek mvdbeek force-pushed the mvdbeek:data_manager_watch_new_tables branch 3 times, most recently from a51f26a to 8f62c97 Sep 15, 2017

mvdbeek added some commits Sep 14, 2017

Make DynamicOptions use updated tool_data_tables
Otherwise a restart would be necessary after adding items to a data
table.

@mvdbeek mvdbeek force-pushed the mvdbeek:data_manager_watch_new_tables branch from c5f385e to 0b7fe43 Sep 15, 2017

@mvdbeek mvdbeek force-pushed the mvdbeek:data_manager_watch_new_tables branch 2 times, most recently from ee3c378 to 195a148 Sep 15, 2017

@mvdbeek mvdbeek force-pushed the mvdbeek:data_manager_watch_new_tables branch from 195a148 to 9d3af63 Sep 16, 2017

shed_data_table_config.write(SHED_DATA_TABLES)

@classmethod
def tearDownClass(cls):

This comment has been minimized.

Copy link
@jmchilton

jmchilton Sep 16, 2017

Member

How about instead of overriding this you put and empty method called _cleanup_galaxy() that matches _prepare_galaxy() in the base class and then just use that here. Might be even cleaner to define a list in parent class called _configuration_files that you can just append to that will get cleaned up automatically. This way you don't need to do anything in this class besides register them and then later on we can make the tests respect GALAXY_TEST_NO_CLEANUP.

This comment has been minimized.

Copy link
@jmchilton

jmchilton Sep 16, 2017

Member

Actually I like this idea so much I already implemented it. If you just append these directories to self._test_driver.temp_directories they should be taken care of automatically. No need to declare a tearDown method.

@jmchilton

This comment has been minimized.

Copy link
Member

commented Sep 16, 2017

This is pretty wonderful!

@mvdbeek mvdbeek force-pushed the mvdbeek:data_manager_watch_new_tables branch from d1b07c9 to 642aaa8 Sep 16, 2017

@scholtalbers

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2017

This will make the data managers a whole lot more useful for quickly adding a new genome! Please release ASAP ;)

@jmchilton jmchilton merged commit a17ae7b into galaxyproject:dev Sep 19, 2017

6 checks passed

api test Build finished. 292 tests run, 5 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 161 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 46 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details
@jmchilton

This comment has been minimized.

Copy link
Member

commented Sep 19, 2017

Great feature, greaterest test! Thanks @mvdbeek this is fantastic!

@jmchilton jmchilton modified the milestones: 18.01, 17.09 Sep 19, 2017

@mvdbeek mvdbeek deleted the mvdbeek:data_manager_watch_new_tables branch Jun 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.