Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ToLoggerConfig() not exporting Caller option #694

Closed
orpheuslummis opened this issue Jul 29, 2022 · 0 comments · Fixed by #696
Closed

ToLoggerConfig() not exporting Caller option #694

orpheuslummis opened this issue Jul 29, 2022 · 0 comments · Fixed by #696
Labels
area/logging Related to the logging/logger system bug Something isn't working
Milestone
DefraDB v0.3

Comments

@orpheuslummis
Copy link
Contributor

https://github.com/sourcenetwork/defradb/blob/develop/config/config.go#L405
@orpheuslummis orpheuslummis added bug Something isn't working area/logging Related to the logging/logger system labels Jul 29, 2022
@orpheuslummis orpheuslummis added this to the DefraDB v0.3 milestone Jul 29, 2022
@jsimnz jsimnz mentioned this issue Jul 29, 2022
Merged
4 tasks
@jsimnz jsimnz closed this as completed in fd88444 Jul 31, 2022
shahzadlone pushed a commit to shahzadlone/defradb that referenced this issue Feb 23, 2024
@jsimnz
fix: Logger not correctly applying named config (sourcenetwork#696)
c3a1cfc 
There was a bug in the sourcenetwork#658 that got merged which effectively broke the entire feature. Tests didn't catch it as they were accidentally poorly defined, as they would pass with the default logger settings as well as the custom ones set in the test.

There are two fixes in this PR that resolves the problem. During review, we decided to switch back the Config.Log from Config.Logger, but the viper Bind calls were still using Bind(logger.loglevel) (old name). Additionally, the validate call on the config.Log struct previously always returned nil (no errors), but was changed recently to validate the available options. But, if the --loglevel option is provided it will set the config.Log.Level to error,defra.cli=info,... for example, which would fail validation. I have no moved the validation rules, as it will be validated anyways later in the call stack (in the ToLoggerConfig).

Lastly, it fixes sourcenetwork#694, as an accidental oversight, potentially during a rebase/merge-conflict.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/logging Related to the logging/logger system bug Something isn't working
Projects
None yet
Milestone
DefraDB v0.3
Development

Successfully merging a pull request may close this issue.

fix: Logger not correctly applying named config sourcenetwork/defradb
1 participant
@orpheuslummis