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

Drop most of the usage of configstate #69

Merged
merged 8 commits into from
Sep 13, 2022

Conversation

natefoo
Copy link
Member

@natefoo natefoo commented Aug 23, 2022

3 properties remain in the configstate in addition to the config file path: config_type, instance_name, and galaxy_root.

This change removes the need for detecting changes between subsequent invocations of Gravity. When update is run, we just generate the new process manager (supervisor) configs based on parsing the current Galaxy config, write out any new or changed configs, and delete everything else. This should be all around simpler and less confusing, plus it fixes the "can't unset env vars" issue I mentioned in #56.

This change will result in parsing at least galaxy.yml and possibly your job config file on most invocations where that would generally only happen on add/register and update before, so that could be a bit slow on a heavily loaded system with slow i/o.

This will be a bit trickier for systemd since our service units won't be in an isolated directory like they are with supervisor, but we might be able to get away with assuming we own everything named /etc/systemd/system/galaxy-*.

Draft momentarily until #68 is merged and I'll rebase that out, plus squash the first 2 commits where I added and removed a tempdir-based method for stateless updates.

@natefoo natefoo requested a review from mvdbeek August 23, 2022 18:58
@natefoo natefoo marked this pull request as ready for review August 29, 2022 15:46
@natefoo natefoo changed the base branch from main to release_1.x September 8, 2022 16:13
@natefoo
Copy link
Member Author

natefoo commented Sep 8, 2022

I have created a new branch for this as I'd like to continue to iterate on the 0.x branch for the rest of 22.05, and this will for the basis of Gravity 1.0 for the 22.09 or 23.0 version of Galaxy.

@hexylena
Copy link
Member

hexylena commented Sep 9, 2022

might be able to get away with assuming we own everything named /etc/systemd/system/galaxy-*.

yeah that feels not unreasonable.

If virtualenv is set in the Gravity config, automatically add its bin dir to $PATH if the gx-it-proxy is enabled
compare with existing configs to determine if changes are needed.
Removes the need for configstate other than path to registered configs.
used by Old Gravity™ many years ago and is incredibly unlikely to exist
in the wild.
@natefoo natefoo merged commit 51c3d52 into galaxyproject:release_1.x Sep 13, 2022
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.

2 participants