-
Notifications
You must be signed in to change notification settings - Fork 615
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.
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
dispatcher: Use Debug instead of Debugf #2282
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2282 +/- ##
==========================================
+ Coverage 60.4% 60.48% +0.08%
==========================================
Files 125 125
Lines 20394 20394
==========================================
+ Hits 12318 12335 +17
+ Misses 6682 6667 -15
+ Partials 1394 1392 -2 |
manager/dispatcher/dispatcher.go
Outdated
@@ -1118,7 +1118,7 @@ func (d *Dispatcher) Session(r *api.SessionRequest, stream api.Dispatcher_Sessio | |||
// get the node IP addr | |||
addr, err := nodeIPFromContext(stream.Context()) | |||
if err != nil { | |||
log.G(ctx).Debugf(err.Error()) | |||
log.G(ctx).Debug(err.Error()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it better to use WithError(err).Debug("")
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated this one to log a descriptive message instead of just spitting out the error.
61963c3
to
d170570
Compare
When the argument is a string returned by Error, we don't want to use Debugf, since if the error string happens to include % characters, this will cause a format string error. Also prefer Debug over Debugf in cases where literal strings do not include any formatting (this part is purely cosmetic). Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
d170570
to
8db89e3
Compare
LGTM |
1 similar comment
LGTM |
- moby/swarmkit#2266 (support for templating Node.Hostname in docker executor) - moby/swarmkit#2281 (change restore action on objects to be update, not delete/create) - moby/swarmkit#2285 (extend watch queue with timeout and size limit) - moby/swarmkit#2253 (version-aware failure tracking in the scheduler) - moby/swarmkit#2275 (update containerd and port executor to container client library) - moby/swarmkit#2292 (rename some generic resources) - moby/swarmkit#2300 (limit the size of the external CA response) - moby/swarmkit#2301 (delete global tasks when the node running them is deleted) Minor cleanups, dependency bumps, and vendoring: - moby/swarmkit#2271 - moby/swarmkit#2279 - moby/swarmkit#2283 - moby/swarmkit#2282 - moby/swarmkit#2274 - moby/swarmkit#2296 (dependency bump of etcd, go-winio) Signed-off-by: Ying Li <ying.li@docker.com> Upstream-commit: 4509a00 Component: engine
- moby/swarmkit#2266 (support for templating Node.Hostname in docker executor) - moby/swarmkit#2281 (change restore action on objects to be update, not delete/create) - moby/swarmkit#2285 (extend watch queue with timeout and size limit) - moby/swarmkit#2253 (version-aware failure tracking in the scheduler) - moby/swarmkit#2275 (update containerd and port executor to container client library) - moby/swarmkit#2292 (rename some generic resources) - moby/swarmkit#2300 (limit the size of the external CA response) - moby/swarmkit#2301 (delete global tasks when the node running them is deleted) Minor cleanups, dependency bumps, and vendoring: - moby/swarmkit#2271 - moby/swarmkit#2279 - moby/swarmkit#2283 - moby/swarmkit#2282 - moby/swarmkit#2274 - moby/swarmkit#2296 (dependency bump of etcd, go-winio) Signed-off-by: Ying Li <ying.li@docker.com> Upstream-commit: 4509a00 Component: engine
When the argument is a string returned by
Error
, we don't want to useDebugf
, since if the error string happens to include % characters, this will cause a format string error.Also prefer
Debug
overDebugf
in cases where literal strings do not include any formatting (this part is purely cosmetic).