Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

fix: log level support #58

Merged
merged 1 commit into from
May 5, 2022
Merged

fix: log level support #58

merged 1 commit into from
May 5, 2022

Conversation

plarsson
Copy link
Contributor

What changed?
Adding an optional flag to set log level.

Why?
Closes #57

How did you test it?
go run ./cmd/temporalite/ start --ephemeral --log-level=info
go run ./cmd/temporalite/ start --ephemeral --log-level=debug
go run ./cmd/temporalite/ start --ephemeral --log-level=info --log-format=pretty
go run ./cmd/temporalite/ start --ephemeral --log-level=debug --log-format=pretty
go run ./cmd/temporalite/ start --ephemeral

Potential risks
log level default (if omitted) is changed to info from debug

Is hotfix candidate?
no

@airhorns
Copy link

airhorns commented May 5, 2022

Would be super great to get this in!

@codecov-commenter
Copy link

Codecov Report

Merging #58 (e5d1095) into main (037b599) will decrease coverage by 1.63%.
The diff coverage is 3.22%.

@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
- Coverage   42.02%   40.38%   -1.64%     
==========================================
  Files          12       12              
  Lines         740      770      +30     
==========================================
  Hits          311      311              
- Misses        407      437      +30     
  Partials       22       22              
Impacted Files Coverage Δ
cmd/temporalite/main.go 1.12% <0.00%> (-0.23%) ⬇️
internal/liteconfig/config.go 81.81% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Collaborator

@jlegrone jlegrone left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

switch c.String(logLevelFlag) {
case "debug", "info", "warn", "error", "fatal":
default:
return cli.Exit(fmt.Sprintf("bad value %q passed for flag %q", c.String(logLevelFlag), logLevelFlag), 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the validation here!

Small nit -- it would be great if the error message included the set of allowed values, with the same source of truth used to generate the log-level flag's usage string. However this isn't high priority since the allowed values should be pretty static, so I'll leave it open to further contribution.

@jlegrone jlegrone merged commit 715fc8d into temporalio:main May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configurable log level
4 participants