ruma / homeserver

A Matrix homeserver written in Rust.
https://www.ruma.io/
1.08k stars 41 forks source link

Allow specifying the configuration file with an environment variable #162

Closed yblein closed 7 years ago

yblein commented 7 years ago

This is a small patch to get my feet wet with Ruma. It addresses #151 by allowing to specify a configuration file through the environment variable $RUMA_CONFIG_FILE. At this point I'm wondering if it is necessary to also add an additional flag since it would be redundant? I was also wondering if there is a reason for the methods of Config to be static instead of having them at the module-level? It seems to introduce an unnecessary level of indirection. Finally, I'm surprised by the number of configuration file formats supported. Is it really worth the additional complexity? Cheers

jimmycuadra commented 7 years ago

Thanks for your interest in the project and for the pull request! Regarding the environment variable and the CLI option, I'd actually like to go the other way and provide this feature only as a CLI option. The main reason is because I plan to update the configuration system to allow for config "versions." This will allow us to make breaking changes to the format of ruma's configuration file in the future, if we need to, by incrementing the version. It also means that the configuration files themselves can be version controlled and diffed by users. Users are still free to use environment variables in any build system to produce the final config file.

The static methods on the Config type were previously top-level functions, and could be again, but putting them under a common type allowed us to clean up imports a bit.

ruma supports multiple configuration file formats because they are all functionally equivalent, each has their own pros and cons, and supporting all of them allows users to pick their preferred format. It also avoids a holy war about which format ruma should support, if there were only one. I think what little complexity this adds is very much worth it.

yblein commented 7 years ago

Ok then. How about we close this and I'll give it another go with a CLI flag?

jimmycuadra commented 7 years ago

Sure, that works.