roadrunner
roadrunner copied to clipboard
[π‘FEATURE REQUEST]: Allow merging two config files
In roadrunner 1.x we could have two different config files, one for production and other for develpment.
An example development config file looked like this:
include:
- .rr.yaml
reload:
interval: 1s
patterns: [".php"]
Now the include block is not handled as spf13/viper does not have this feature implemented, (but is requested https://github.com/spf13/viper/pull/893).
As a workaround for this problem I can suggest two solutions:
- Allow passing
--configmultiple times on the command line and merge the configuration from multiple files. - Add a key
enabledunder reload, so I can call the server with-o reload.enabled=falsein production.
FYI, we implemented this feature in RR 1 itself. Not via Viper.
The ticket in the Discuss stage, so we can discuss, what option fits us and the community better.
I guess that we should wait for the viper PR merged and try to implement let's say "native" includes. Also, we had a discussion, that will be good to have enabled option in every plugin.
I would like having an enabled option in every plugin just because of the symmetry. This on it's own would suffice to have reload disabled in production.
But I would also like to be able to have very different logging configurations in each environment and this can be cumbersome with the -o options, so having an include mechanism would be the optimal solution.
@hugochinchilla Yeah, I agree to include enabled keys to every plugin. As for the 'includes', let's wait for some time for the viper ticket to get merged (let's say 1-2 weeks) and then return to the discuttion. I plan it to 2021-05-11 release cycle.
@rustatian Hello! viper ticket was closed https://github.com/spf13/viper/pull/893#issuecomment-890258466
"Allow merging config files" will come in viper v2. Maybe. Milestone for v 2.0 looks abandoned
Hey @embargo2710 ππ» Yeah, I saw this πΏ It might be an excellent feature to avoid some boilerplate configuration for the dev/prod/staging/etc.
But, to say in truth, I might implement this without waiting for the viper v2 π Just come to our open planning session (RR streams channel in our discord server) and send this ticket π We will discuss it and prioritize.
What about include feature? I want this)
Hey @zolotov88 ππ» Hit the ππ» button π Join our discord, attend the RR planning session (we do them online at twitch.tv/rustatian) and bring this ticket, we will discuss it and include it in some of the upcoming sprints. β‘
Where I can find your discord?
---> https://github.com/roadrunner-server
@rustatian Thank you!
@rustatian Maybe there is still a time in the next release for this task?
@Kaspiman I guess this is fairly easy to implement:
- Introduce a new key:
includewith the paths. - Use temporary instances of the
viperto read configurations by paths. - Use
AllKeysmethod (or other which allow you iterating over the key-vals) in every tmpviperinstance. While iterating, set key-val pairs to the original configuration usingSetmethod. - This should be done before this line: https://github.com/roadrunner-server/config/blob/master/plugin.go#L71
- Done π
As for the p.2 and temporary instances: I don't know if that possible to add configurations to the single instance (as far as I know - no).
This feature will be in the next release under the -e (experimental) RR flag.
Maybe I'm late, but based on a rich experience working with complex docker compose yaml configurations, I would expect the following merging functionality (based on the existing Docker compose functionality and requested by the community):
- Override and extend any part of the config, including nested ones (both key-value and ordinal entries). I.e. I can add a new named value to anywhere, I can add a new item to an ordinal array.
- Replace the entire array - both named and ordinal. I.e. I can replace some entire section or whole array without overriding every existing value.
- Delete the entire array - both named and ordinal. I.e. I can delete some section or array as if it was never declared.
Hey @speller :wave: Documentation for this experimental feature is here: https://roadrunner.dev/docs/experimental-experimental/current/en
- With the included config, you can override any of the root configuration values.
- Also supported.
- Not supported.
Guys, while the issue is closed, but the discussion is not locked, feel free to send your feedback here or on our Discord server [RR channel].
@rustatian Unfortunately, it doesn't work as expected for me. This is why I've wrote down my expectation in the details. My use case as an example:
I have a "production" rr.yaml file, then, in the dev move, I want to override/add/change something without duplicationg the code. I try the following command:
rr serve -e -c .rr.yaml -o include=".rr.dev.yaml"
With the following dev config:
version: '3.8'
server:
env:
- XDEBUG_TRIGGER: 1
And I get the following error:
handle_serve_command: Init error:
server_plugin_init: command should not be empty
So the extension or merging of config doesn't work as the majority of users who have experience working with docker compose yaml configs and with Symfony yaml configs will expect.
Here I want to add just a single environment variable and it breaks the whole config making RR not working at all.
Also, I would expect passing multiple config files either this way:
rr serve -c .rr.yaml -c .rr.dev.yaml
or
rr serve -e -c .rr.yaml .rr.dev.yaml
without unintuitive workarounds with include.
Please treat it as feedback, not as blame.
Additionally, I would expect a way to control the resulting configuration after merging - a dump function, or printing it to the console output on startup, or anything else. To see what RR got after its internal processing and is it what I intended to have.
Hey @speller π
Please treat it as feedback, not as blame.
Sure π
What is in your .rr.yaml file? I mean, server section.
@rustatian It's a very common PHP section from samples or documentation:
server:
command: "php index.php"
env:
- APP_RUNTIME: Baldinof\RoadRunnerBundle\Runtime\Runtime
@speller, Yeah, only env is different, and thus it should be applied. I guess this is just how viper (library I use under the hood) works. I'll check what I can do with that.