-
Notifications
You must be signed in to change notification settings - Fork 1k
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
ease the usage of galaxy.yml in ansible #8252
Conversation
the sample file is not directly modified but rather generated via https://github.com/galaxyproject/galaxy/blob/dev/Makefile#L94 from https://github.com/galaxyproject/galaxy/blob/dev/doc/source/admin/galaxy_options.rst |
Damn! 😝 |
uwsgi has very particular YAML processing - I don't think this would work - would it? |
@jmchilton It seems so! BTW, I didn't find the time yet to implement that using your procedure. |
@nuwang I thought you might have thoughts on this or advise for @lecorguille |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mvdbeek Thanks for bringing this up. My thoughts on this now are that it would be strongly desirable to move the uwsgi config to a separate file as you found was possible last time, because it really isn't yaml and makes the galaxy config file unmodifiable programatically, which is a bad situation to be in. Since it's relatively less likely that someone has modified the defaults in uwsgi.yaml, I think that this could be done with comparatively less disruption. It works quite well in the k8s setup.
@lecorguille The issue that will cause this to fail is the difference in how yaml arrays are defined here:
link
And here:
static-map: /static/style=static/style/blue |
The repeating static-map in the latter case is invalid yaml, and will cause only the last entry to be included when you use a yaml anchor.
The reason this works with ansible-galaxy is because it has special logic to convert the valid yaml to invalid yaml that uwsgi understands:
https://github.com/galaxyproject/ansible-galaxy/blob/f450af8a6b8ad5c7c400603f4eed135810c8d81f/filter_plugins/uwsgi_yaml.py#L55
And even the libyaml version of uwsgi still doesn't use valid yaml: unbit/uwsgi#2097
so this is unlikely to be resolved in the short run.
Instead, what if you modify this line to use a jinja2 combine instead of a yaml anchor? That way, you might still be able to do what you're proposing (which is very useful) without needing to modify the sample config at all.
Given @nuwang's suggestion I'm going to close this one, but thanks for this nice idea @lecorguille! |
Using anchor, you just need to modify the galaxy.yml and the ansible-galaxy role will do the almost rest. Thus, it's easier to sync the galaxy.yml after an update to the instance!?