Configuring startup parameters from environment. #71
Conversation
.gitignore
Outdated
| @@ -2,3 +2,10 @@ vendor/ | |||
| cmd/commento/commento | |||
| *.db | |||
| .idea | |||
| .env | |||
There was a problem hiding this comment.
you can use .env* here instead of manually specifying each suffix, will allow ignoring any other .env's used
There was a problem hiding this comment.
@DarkThrone mentioned having a default environment that developers can use out of the box so there is one environment file that is not gitignored and that is .env.development.
Because I allow one of the 8 or 9 environments to be checked in, I had to specify the others individually.
There was a problem hiding this comment.
You can still use the .env*, then exclude the single one to allow with ! e.g. !.env.notignored
cmd/commento/main.go
Outdated
| @@ -15,23 +16,23 @@ func main() { | |||
| if err != nil { | |||
| Die(err) | |||
| } | |||
|
|
|||
|
|
|||
| // Load configuration from the environment. | |||
There was a problem hiding this comment.
The env file can be specified explicitly e.g. via a env variable or a flag, and default to .env .
e.g.
A global $COMMENTOENV=development.local will provide specificity, where the env file will be ".env."+os.Getenv("COMMENTOENV"), and preventing best-guess config selection using this loop.
There was a problem hiding this comment.
The godotenv comes from the Ruby world where they have the 'convention over configuration' outlook. The order of searching through the environments is specified by the ruby package but not built into godotenv yet. I'm going to contact the developer to nudge him to add the functionality.
cmd/commento/main.go
Outdated
| } | ||
|
|
||
| if demoEnv := os.Getenv("DEMO"); demoEnv == "true" { | ||
| if demoEnv := os.Getenv("COMMENTO_DEMO"); demoEnv == "true" { |
There was a problem hiding this comment.
how about if os.Getenv("COMMENTO_DEMO") == "true" {
There was a problem hiding this comment.
That's not bad. What if the user enters 'truee' or 'fals' or has some other writing error? We should probably check that a COMMENTO_DEMO actually parses to a boolean:
if s, err := strconv.ParseBool(os.Getenv("COMMENTO_DEMO")); err != nil {
Logger.Infof("COMMENTO_DEMO not set properly, defaulting to no demo")
}
There was a problem hiding this comment.
In this case we're literally looking for the string "true" and not a bool, as its returned by os.Getenv. Basically =="true" would be the best way to check for it, hence ignore anything else
cmd/commento/main.go
Outdated
| @@ -6,6 +6,7 @@ import ( | |||
| "time" | |||
|
|
|||
| _ "github.com/mattn/go-sqlite3" | |||
| "github.com/joho/godotenv" | |||
|
|
|||
| . "github.com/adtac/commento/lib" | |||
There was a problem hiding this comment.
I know this isn't part of this commit, but dot imports are not nice. From golang.org: We do not recommend the use of import . outside of tests, and using it may cause a program to fail to compile in future releases.
There was a problem hiding this comment.
I actually like those dot imports when they are used sparingly but, I recognize they are a discouraged practice. Almost all (probably all) dot imports will be removed eventually.
I include the dot imports to avoid changing too much of the code at once.
Without the dot import there would be, for example, references to
lib.LoadDatabase instead of LoadDatabase. But if the package changes then all the libs have to change to that new package name which is annoying.
When the structure of the code is more settled the dots will get removed. Really, they are just there now for ease of readability (that's why I like them even if I know they have to go...)
|
@GaiusMartius it looks like the commit author information is different here? Do you still wanna go ahead with that email ID or do you want to amend that? |
GaiusMartius
force-pushed
the
GaiusMartius/LoadingConfigFromEnvironment
branch
from
4ec57a5
to
4b3408c
Compare
cmd/commento/main.go
Outdated
|
|
||
|
|
||
| // Load configuration from the environment. | ||
| // Values in earlier files will take precedence over later values |
There was a problem hiding this comment.
you mean "values in later files will take precedence" right? Because a setting in .env.development.local will be overwritten if the same setting has a different value in .env, which mean .env takes precedence over .env.development.local
There was a problem hiding this comment.
Earlier files do take precedence. If you have a .env.development.local, it is assumed that you want those values to be primary because you're developing.
Deployments would just contain only the .env they intend to use, like .env.production
However, if you do not set a value then later files will set the value. The first one to set wins.
GaiusMartius
force-pushed
the
GaiusMartius/LoadingConfigFromEnvironment
branch
from
4b3408c
to
153ffe0
Compare
|
@GaiusMartius no merge commits please: I prefer to have a linear master commit history. |
GaiusMartius
force-pushed
the
GaiusMartius/LoadingConfigFromEnvironment
branch
3 times, most recently
from
46a9b46
to
113d1d2
Compare
cmd/commento/.env.development
Outdated
| @@ -0,0 +1,2 @@ | |||
| COMMENTO_PORT=8080 | |||
| COMMENTO_DEMO=false | |||
GaiusMartius
force-pushed
the
GaiusMartius/LoadingConfigFromEnvironment
branch
from
113d1d2
to
0889181
Compare
GaiusMartius
force-pushed
the
GaiusMartius/LoadingConfigFromEnvironment
branch
from
fde9e95
to
dbdb0a2
Compare
|
Cherry picked in #88 |
|
Merged through #88 |
Defaults are included in cmd/commento/.env.development