feat: make logger easily replaceable #15358
Conversation
github-prbot
requested review from
a team,
aaronc and
atheeshp
and removed request for
a team
| @@ -1,5 +1,5 @@ | |||
| --- | |||
| sidebar_position: 0 | |||
| sidebar_position: 1 | |||
There was a problem hiding this comment.
unrelated: #15354 (comment)
| @@ -51,20 +50,11 @@ type Context struct { | |||
| Logger log.Logger | |||
| } | |||
|
|
|||
| // ErrorCode contains the exit code for server exit. | |||
server/util.go
Outdated
| @@ -116,25 +125,25 @@ func bindFlags(basename string, cmd *cobra.Command, v *viper.Viper) (err error) | |||
| // is used to read and parse the application configuration. Command handlers can | |||
| // fetch the server Context to get the CometBFT configuration or to get access | |||
| // to Viper. | |||
| func InterceptConfigsPreRunHandler(cmd *cobra.Command, customAppConfigTemplate string, customAppConfig interface{}, cmtConfig *cmtcfg.Config) error { | |||
| func CreateServerContextFromConfig(cmd *cobra.Command, customAppConfigTemplate string, customAppConfig interface{}, cmtConfig *cmtcfg.Config) (*Context, error) { | |||
There was a problem hiding this comment.
Do you think @tac0turtle this conveys enough what the function does? (which is more than creating a server context, as it calls interceptConfigs).
I feel like maybe InterceptConfigsPreRunHandlerNoLoggerNoSaveContext, while hideous, will convey more what this does. I am open for a better name too 😄.
There was a problem hiding this comment.
Actually, I even think, this could be named InterceptConfigsPreRunHandler and have the other one be renamed to something more clear. However doing that is API breaking, so not ideal.
There was a problem hiding this comment.
Updated to InterceptConfigsAndCreateContext.
julienrbrt
changed the title
| default: | ||
| os.Exit(1) | ||
| } | ||
| panic(err) |
There was a problem hiding this comment.
Does this keep the exit code of 1? If not, please revert.
Description
Closes: #15341
This creates an alternative to
InterceptConfigsPreRunHandlerwhich does not configure a logger and does not set the server context.This allows apps to keep using
InterceptConfigsPreRunHandlerfrom the SDK to keep the same behavior, or use the alternative function to configure their logger, without having to copy the whole server packages (due to some internal functions).Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change