-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
Improve error message for conflicting container name. #9765
Conversation
@@ -478,7 +478,7 @@ func (daemon *Daemon) reserveName(id, name string) (string, error) { | |||
} else { | |||
nameAsKnownByUser := strings.TrimPrefix(name, "/") | |||
return "", fmt.Errorf( | |||
"Conflict, The name %s is already assigned to %s. You have to delete (or rename) that container to be able to assign %s to a container again.", nameAsKnownByUser, | |||
"Conflict, The name %s is already assigned to %s. You have to delete that container to be able to assign %s to a container again.", nameAsKnownByUser, |
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.
Just my 2cts as a non native English speaker, but I'd probably seize the opportunity to rephrase and make it simpler (cc @SvenDowideit):
"Name %s is already assigned to container %s: you must delete that container to be able to reuse that name."
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.
@icecrime yes it could be improved overall. Decided to just fix the confusing part for now, but I'm happy to improve further if desired. |
This changes the error message that is returned by the daemon when a container-name already exists. The old message suggests that containers can be renamed, which is currently not possible. To prevent confusion, the part "(or rename)" is removed from the error-message. Message before this change; FATA[0000] Error response from daemon: Conflict, The name foobar is already assigned to 728ac36fb0ab. You have to delete (or rename) that container to be able to assign foobar to a container again. Message after this change; FATA[0000] Error response from daemon: Conflict. The name 'foobar' is already in use by container 728ac36fb0ab. You have to delete that container to be able to reuse that name. Relates to: moby#3036 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
b85a913
to
aa9c956
Compare
/ping @icecrime @SvenDowideit PTAL. I rewrote the message a bit, but open to better suggestions. I also used %q for the name, which should put it in single quotes to make the "name" stand out slightly better. wrt the "Conflict."; unfortunately I cannot remove that, because that's a magic string used in https://github.com/docker/docker/blob/master/api/server/server.go#L127. It's called "plain text programming" and really should be find an alternative for 😄 |
Nice catch! I'm hesitating to punctuate with a 😄 or a 😭 We could hide it inside the sentence, like LGTM |
Yes, luckily I remembered that (it came up in another issue). Perhaps I should add a note to prevent a future 'visitor' to change it by mistake. wdyt? |
Nice :) and I did a grep - that old string isn't mentioned elsewhere. LGTM |
LGTM |
Improve error message for conflicting container name.
Thanks! |
This changes the error message that is returned by the daemon when
a container-name already exists.
The old message suggests that containers can be renamed, which is
currently not possible.
To prevent confusion, the part "(or rename)" is removed from
the error-message.
Message before this change;
Message after this change;
Relates to: #3036
Signed-off-by: Sebastiaan van Stijn github@gone.nl