Repository navigation
feat(storage): reading env file - #61
DreamAndDrum wants to merge 10 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #61 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 69 69
Lines 4618 4645 +27
=========================================
+ Hits 4618 4645 +27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| var cfg config.Config | ||
|
|
||
| filename := flag.String("config", "", "path to configFile") | ||
| if filename == nil || *filename == "" { |
There was a problem hiding this comment.
you should use this approach
filename == nil || filename == nil && *filename == ""
|| will mean it can check if nil pointer == ""
There was a problem hiding this comment.
also you can remove else and just have
cfg = config.New() by default
and replace if with
if !(filename == nil || filename == nil && *filename == "") { cfgFromFile, err := readConfigFile(filename)
There was a problem hiding this comment.
Yea, I was thinking about that, and then I supposed, that it's better to make behaviour more explcict. As user I would prefer to know, if my config is read from os.env, for the case when I've forgotten to mention this flag.
| logger.Fatal(storageServ.Run().Error()) | ||
| } | ||
|
|
||
| func readConfigFile(filename *string) (config.Config, error) { |
There was a problem hiding this comment.
it would be better to just have parameter filename string (since you already checked it)
| } | ||
| } | ||
|
|
||
| dbType := DB_TYPE_CLICKHOUSE |
There was a problem hiding this comment.
os.getenv has default option
os.Getenv(ENV_DB_LOGS, DB_TYPE_CLICKHOUSE)
There was a problem hiding this comment.
Hm, in default os package it is func Getenv(key string) string
There was a problem hiding this comment.
@DreamAndDrum ahh, anyways it can be a small local function
| } | ||
|
|
||
| // validate config | ||
| if config.DbHost == "" { |
There was a problem hiding this comment.
it should be possible to automate these checks, can cause issues down the road
No description provided.