Visitar URL original
feat(storage): reading env file by DreamAndDrum · Pull Request #61 · squzy/squzy · GitHub
Skip to content

feat(storage): reading env file - #61

Open
DreamAndDrum wants to merge 10 commits into
developfrom
storage-config-yaml
Open

DreamAndDrum wants to merge 10 commits into
developfrom
storage-config-yaml

Conversation

@DreamAndDrum

Copy link
Copy Markdown
Member

No description provided.

@codecov

codecov Bot commented Jan 16, 2022 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (55c6a59) to head (f596002).
⚠️ Report is 115 commits behind head on develop.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread apps/squzy_storage/main.go Outdated
var cfg config.Config

filename := flag.String("config", "", "path to configFile")
if filename == nil || *filename == "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should use this approach
filename == nil || filename == nil && *filename == ""

|| will mean it can check if nil pointer == ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {

@peterjasc peterjasc Jan 16, 2022 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be better to just have parameter filename string (since you already checked it)

}
}

dbType := DB_TYPE_CLICKHOUSE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.getenv has default option

os.Getenv(ENV_DB_LOGS, DB_TYPE_CLICKHOUSE)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, in default os package it is func Getenv(key string) string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DreamAndDrum ahh, anyways it can be a small local function

}

// validate config
if config.DbHost == "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be possible to automate these checks, can cause issues down the road

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants