SNOW-625282: Make logrus dependency optional or drop it all together #1286
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
| params := parseParameters("parameters.json", "testconnection") | ||
|
|
||
| // get environment variables | ||
| env := func(key, defaultValue string) string { | ||
| if value := os.Getenv(key); value != "" { | ||
| return value | ||
| } | ||
| if value, ok := params[key].(string); ok && value != "" { | ||
| return value | ||
| } |
There was a problem hiding this comment.
this change is not necessary for the PR but it does allow parameters to be read from the json file as per the README.
| @@ -149,7 +149,6 @@ func TestLogToConfiguredFile(t *testing.T) { | |||
|
|
|||
| logger.Error("Error message") | |||
| logger.Warn("Warning message") | |||
| logger.Warning("Warning message") | |||
There was a problem hiding this comment.
Warning is basically an alias for Warn so removing it since it clutters the interface otherwise.
| func (log *defaultLogger) Fatalf(format string, args ...interface{}) { | ||
| if log.enabled { | ||
| log.inner.Fatalf(format, args...) | ||
| } | ||
| } | ||
|
|
||
| func (log *defaultLogger) Panicf(format string, args ...interface{}) { | ||
| if log.enabled { | ||
| log.inner.Panicf(format, args...) | ||
| } | ||
| } |
There was a problem hiding this comment.
all Fatal and Panic forms removed from the interface, since they were only used in testing code.
| func (log *defaultLogger) TraceFn(fn rlog.LogFunction) { | ||
| if log.enabled { | ||
| log.inner.TraceFn(fn) | ||
| } | ||
| } |
There was a problem hiding this comment.
all Fn variants removed; again unused in prod code.
| // AddHook adds a hook to the logger hooks. | ||
| func (log *defaultLogger) AddHook(hook rlog.Hook) { | ||
| log.inner.AddHook(hook) | ||
|
|
||
| } | ||
|
|
||
| // IsLevelEnabled checks if the log level of the logger is greater than the level param | ||
| func (log *defaultLogger) IsLevelEnabled(level rlog.Level) bool { | ||
| return log.inner.IsLevelEnabled(level) | ||
| } |
| func (log *defaultLogger) SetFormatter(formatter rlog.Formatter) { | ||
| log.inner.SetFormatter(formatter) | ||
| } | ||
|
|
||
| // SetOutput sets the logger output. | ||
| func (log *defaultLogger) SetOutput(output io.Writer) { | ||
| log.inner.SetOutput(output) | ||
| } | ||
|
|
||
| func (log *defaultLogger) SetReportCaller(reportCaller bool) { | ||
| log.inner.SetReportCaller(reportCaller) | ||
| } | ||
|
|
There was a problem hiding this comment.
SetFormatter / SetReportCaller only have 1 internal usage and do not need to be part of the interface.
|
I have read the CLA Document and I hereby sign the CLA |
Description
The following changes decouple logrs from SFLogger, with the eventual goal to allow per context logging control (in a followup PR) by a user provided SFLogger.
This PR is not (yet) complete but is presented as a proof-of-concept. Further cleanup will be performed if this approach is deemed acceptable. One thing to be aware of is that certain interfaces are no longer supported in particular the functional variants of logging, and the WithTime, WithFields, WithError, Panic and Fatal which were all ONLY being exercised in unit tests.
Checklist