Resource Dispatcher Config V2 #59
Conversation
…eeds documentation)
|
(draft status until I update the documentation for these changes as well) |
|
@oybed ping: this PR should be ready for review when you get a chance - other small changes I have written down for the dispatcher are for the Ansible Tower plugin and don't really fit in with this PR. |
| configuration["repositories"].extend(config_tmp["repositories"]) | ||
| if "tasks" in config_tmp: | ||
| configuration["tasks"].extend(config_tmp["tasks"]) | ||
| print(f"Loaded configuration file: {path}") |
There was a problem hiding this comment.
A couple of suggestions that will perhaps help with troubleshooting later on:
- make sure to indicate that his was in the
dirmode - i.e.: right now this line is the same as 23 above (yes, it will show the path, but still good to differentiate the actual message IMHO). Perhaps also add a message earlier on (approx line 32) indicating that it is going down the directory path - If a file is skipped due to not ending in
.ymlor.yaml, perhaps indicate so (?) - unless the content is
repositoriesortasks, it won't be consumed - perhaps indicate that to the user (catches typos, etc.)
related to #2 - the file mode above doesn't do any .yml or .yaml checks - perhaps it should (to be consistent with directory processing)?
| @@ -1,6 +1,8 @@ | |||
| # Deployment | |||
|
|
|||
| This is fundamentally just a Python project that versions dependencies in `Pipenv` & `Pipenv.lock` files. Any system that can run projects using Pipenv should be able to run this application. If you are running this application standalone, you will need to set the environment variable `CONFIG_FILE` to the path of the config file to use. That said, there is a Helm chart available to get up and running quickly on OpenShift. | |||
| This is fundamentally just a Python project that versions dependencies in `Pipenv` & `Pipenv.lock` files. Any system that can run projects using Pipenv should be able to run this application. If you are running this application standalone, you will need to set either the environment variable `CONFIG_FILE` to the path of the config file to use, or `CONFIG_DIR` to a directory of configuration YAML files to load. | |||
There was a problem hiding this comment.
Not sure which README it should go, but would be good to include a bit more information on the directory option, e.g.:
- does it only sources
.ymland.yamlfiles found at the dir level - i.e.: no sub-directories - the merge of the files is limited to
repositoriesandtasks(?) - the intended purpose of the merge - i.e.: does/does not merge values of an object(?)
Some of the same goes for the deep_merge part, so functionality relying on it could perhaps be mentioned in the same context as well
There was a problem hiding this comment.
I have significantly rethought the whole config merge strategy and written more documentation about it... Please see what you think now.
I'd like to somewhat avoid the user having to think too hard about how the merge works while working on configs, so merge issues that will definitely cause a problem (like name or route collisions) are now specifically detected and warned against. Merge issues that will not cause a problem (like an identical repository dependency in two config files) are silently handled. Please let me know what you think of moving towards this more targeted config merge strategy (deep_merge is still around, just used for some more internal, runtime-level functions).
…ate single file & directory config init message, notify when ignoring files in the directory, and warn when a config file has extraneous parameters.
What does this PR do?
Revamp of dispatcher's configuration system. Now supports jinja2 templating, variable injection via environment vars, and multiple config files for better organization of the configs.
How should this be tested?
Try to use multiple config files and/or config files with env vars in them 🙂
Is there a relevant Issue open for this?
Other Relevant info, PRs, etc.
People to notify
cc: @redhat-cop/tool-integrations