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

More structured app shutdown. #4910

Merged
merged 1 commit into from Nov 1, 2017

Conversation

Projects
None yet
5 participants
@jmchilton
Member

jmchilton commented Nov 1, 2017

  • Cleanup database engine when shutting down Galaxy app (this wasn't done previously and I think it can cause connection issues when testing across different databases).
  • Join monitor threads when shutting down (old behavior can be restored by setting wait time for monitor threads back to "0"). See note in galaxy.ini.sample.
  • Better error handling during shutdown, don't let a failure to cleanup one thing cause other things not to be cleaned up.
  • New monitor thread mixin based on jobs stuff - reuse for workflows so it properly wakes up workflow scheduling on shutdown.
More structured app shutdown.
- Cleanup database engine when shutting down Galaxy app (this wasn't done previously and I think it can cause connection issues when testing across different databases).
- Join monitor threads when shutting down (old behavior can be restored by setting wait time for monitor threads back to "0"). See note in galaxy.ini.sample.
- Better error handling during shutdown, don't let a failure to cleanup one thing cause other things not to be cleaned up.
- New monitor thread mixin based on jobs stuff - reuse for workflows so it properly wakes up workflow scheduling on shutdown.

@galaxybot galaxybot added this to the 18.01 milestone Nov 1, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Nov 1, 2017

Rollback isolated, templated databases for integration tests.
Integration tests are broken in dev - they can be fixed by merging galaxyproject#4914 and galaxyproject#4910 or by this commit. If those PRs are merged first (my preference) I'll just close this one. If not, I'll open a fourth PR to roll this back after those are merged.

@dannon dannon merged commit 7d1bdb2 into galaxyproject:dev Nov 1, 2017

6 checks passed

api test Build finished. 306 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 162 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 57 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
self.control_worker.shutdown()
except Exception as e:
exception = exception or e
log.exception("Failed to shutdown control worker cleanly")
except AttributeError:
# There is no control_worker

This comment has been minimized.

@nsoranzo

nsoranzo Nov 1, 2017

Member

This will never be executed because an AttributeError would be caught by the internal except.

This comment has been minimized.

@jmchilton

jmchilton Nov 2, 2017

Member

Yes... good point. I'll see what I can do.

@jmchilton jmchilton referenced this pull request Nov 2, 2017

Merged

Run Galaxy fully under uWSGI, including job handlers #4475

14 of 14 tasks complete

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Nov 2, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Nov 2, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Nov 2, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Nov 2, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Nov 2, 2017

@natefoo

This comment has been minimized.

Member

natefoo commented on lib/galaxy/util/monitors.py in 09c9f27 Nov 17, 2017

Is this correct when start = False?

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