drogon
drogon copied to clipboard
Add dotenv config support
- Fix #1184
Thanks for the PR! May you explain the use case for this any why not Json?
Thanks for taking the time to review the PR.
Now, let's say we have a server running in Docker, a very common use case would be a docker-compose file that configures the server and the database and maybe a cache database and a proxy server, the question is, how can I share the config between the docker containers and the Drogon server, config like the database host and port.
Given the example above a good solution would be using a .env
file since Docker already have built-in support to reading config from a .env
file.
An example config structure would be like that:
- docker-compose.yaml
services:
drogon:
image: drogon
ports:
- ${HTTP_PORT}:${HTTP_PORT}
- ${HTTPS_PORT}:${HTTPS_PORT}
redis:
image: redis
ports:
- ${REDIS_PORT}:${REDIS_PORT}
postgres:
image: postgres
environment:
POSTGRES_DB: ${POSTGRES_DB}
POSTGRES_USER: ${POSTGRES_USER}
ports:
- ${POSTGRES_PORT}:${POSTGRES_PORT}
- .env
HTTP_PORT=80
HTTPS_PORT=443
REDIS_PORT=6379
POSTGRES_DB=postgres
POSTGRES_USER=postgres
POSTGRES_PORT=5432
Now, if Drogon has build-in support for the .env
file one will be able to share the config between database, cache, proxy, etc. from a single source which is the .env
file.
An alternative to reading the Drogon config from the .env
file would be reading/parsing the .env
file and assigning the variable to the environment variables and then use that instead, which is actually how .env
files was designed to work.
If that alternative makes more since I can update the implementation to do it that way instead!
@ahmedmkamal Thanks for the quick and detailed reply. Please give me and @an-tao some time to think through it. It's quite a big change and has a lot of implications.
Generally speaking I like the idea, but I think we need a more generic implementation in order to be mergeable into main.
There’s also still the question open, how exactly “environment” variables should be supported, i. e. is this feature about INI file support or environment variables support?
@rbugajewski It's about environment variables support, not INI, actually the .env
file is not an INI file it's just a plain text file that declares environment variables in the form of VAR=value
it may be a valid subset of INI but it's not fully compatible.
I've been looking into my implementation (trying different solutions) over the weekend and I found that this problem can be split into two parts...
- Adding support to parsing a
.env
file and assigning the parsed values to the "environment variables" ONLY if the variable is not already assigned. - Adding support to reading all configuration parameters from environment variables.
Regarding the format of the .env
file, after looking into how other software is parsing the .env
file (specifically Docker) I found out that to be compatible with other software the parsing rules will be as follows:
- variables are assigned in the form of
VAR=value
separated by a new line. - anything that comes after a
#
sign is a comment unless it's wrapped in quotes. - empty values are treated as empty strings
VAR=
equalsVAR=""
. - white spaces are trimmed unless it's wrapped in quotes.
- namespaces
[NAMESPACE]
are not supported.
Side Note: I'm usually referencing Docker in this context because one of the main use cases of this feature will be sharing the configuration with Docker or rather reading the configuration parameters from environment variables that are being assigned with Docker.
@rbugajewski It's about environment variables support, not INI, actually the
.env
file is not an INI file it's just a plain text file that declares environment variables in the form ofVAR=value
it may be a valid subset of INI but it's not fully compatible.I've been looking into my implementation (trying different solutions) over the weekend and I found that this problem can be split into two parts...
Thanks for the clarification. So if Docker’s .env
files are a subset of INI files, I think we could reuse an existing library instead of reinventing the wheel (e. g. iniparser is license-compatible to Drogon, and has Chinese & English documentation, so in some way it fits into this project). I would also think it is better to have this dependency optional as I don’t see it as a core feature required for the framework to work. What do you think @an-tao, @marty1885?
Regarding the format of the
.env
file, after looking into how other software is parsing the.env
file (specifically Docker) I found out that to be compatible with other software the parsing rules will be as follows:
- variables are assigned in the form of
VAR=value
separated by a new line.- anything that comes after a
#
sign is a comment unless it's wrapped in quotes.- empty values are treated as empty strings
VAR=
equalsVAR=""
.- white spaces are trimmed unless it's wrapped in quotes.
This is basically how almost every INI parser works.
- namespaces
[NAMESPACE]
are not supported.
I’m afraid this is incompatible with Drogon. How would you map the environment variables to a real configuration without sections, @ahmedmkamal?
Side Note: I'm usually referencing Docker in this context because one of the main use cases of this feature will be sharing the configuration with Docker or rather reading the configuration parameters from environment variables that are being assigned with Docker.
I think it’s a great use case and it saves some work. It’s a pragmatic feature and I like the concept 👍
I just had a completely different thought: Maybe it would be better for a first step to extend drogon_ctl
with a subcommand to generate a .env
file from a JSON configuration?
This could be easily automated.
@rbugajewski
I’m afraid this is incompatible with Drogon. How would you map the environment variables to a real configuration without sections?
maybe using DROGON_SSL_CERT=path/to/cert
for the parameter { "ssl": { "cert": "path/to/cert" } }
and DROGON_APP_NUM_THREADS=0
for { "app": { "number_of_threads": 0 } }
etc... do you think that will work?
I just had a completely different thought: Maybe it would be better for a first step to extend
drogon_ctl
with a subcommand to generate a.env
file from a JSON configuration?This could be easily automated.
I believe I can do that, should I start working on it?
I’m afraid this is incompatible with Drogon. How would you map the environment variables to a real configuration without sections?
maybe using
DROGON_SSL_CERT=path/to/cert
for the parameter{ "ssl": { "cert": "path/to/cert" } }
andDROGON_APP_NUM_THREADS=0
for{ "app": { "number_of_threads": 0 } }
etc... do you think that will work?
I think in this case there will be issues to create a generalized mapping, because you will have to somewhere encode that DROGON_APP_NUM_THREADS
maps to app
, then to number_of_threads
.
Even if we would add some heuristic this would be an issue. You can’t know if APP_NUMBER_OF_THREADS
maps to app_number
and of_threads
, or app
and number_of_threads
.
We could introduce a separator like double underscore (APP__NUMBER_OF_THREADS
). But this could easily break, and would look unmanageable in the configuration file.
I think we need an intermediate layer, consider the scenario of adding a new option, if we don't have an intermediate layer, we have to add support for this in json parser and env parser.
json parser ---------
evn parser -------------- intermediate layer-------drogon::app().configInterface()
yml parser ----------
...