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.

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

Merged
merged 1 commit into from
Jun 22, 2017
Merged

Conversation

aaronlehmann
Copy link
Collaborator

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).

@codecov
Copy link

codecov bot commented Jun 22, 2017

Codecov Report

Merging #2282 into master will increase coverage by 0.08%.
The diff coverage is 60%.

@@            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

@@ -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())
Copy link
Contributor

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("")?

Copy link
Collaborator Author

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.

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>
@ijc
Copy link
Contributor

ijc commented Jun 22, 2017

LGTM

1 similar comment
@cyli
Copy link
Contributor

cyli commented Jun 22, 2017

LGTM

@cyli cyli merged commit e15198c into moby:master Jun 22, 2017
andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request Jul 14, 2017
- 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
silvin-lubecki pushed a commit to silvin-lubecki/engine-extract that referenced this pull request Mar 16, 2020
- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants