Fix bug which mistakes 400 error for 500 #22762
Conversation
|
it's make sense to return 400 if no command specified, so LGTM |
|
Will this be a breaking change for people using the API? Technically, it's a change in the API, so it's possible this change should be versioned, but perhaps it's not that big of a problem (so, just mentioning it). ping @vdemeester @calavera @MHBauer wdyt? |
|
Is this on the create or the start? Either way I think we need an update to the docker remote api docs, https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api_v1.24.md, as, @thaJeztah says, it is an API change. |
|
It should be documented, but it is unlikely to break anything in practise, looks good. |
|
Alright, let's go with documenting it; @coolljt0725 can you add this change to https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api.md#v124-api-changes, and to https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api_v1.24.md. Given that we are not going to version this change, all older versions of the API docs should also be updated |
|
@thaJeztah ok, will do |
|
Oh! @coolljt0725 I actually meant to ping @wangxing1517 😊 |
|
@thaJeztah haha, it's ok :) |
|
@thaJeztah , I am done with my work. |
| @@ -116,6 +116,7 @@ This section lists each version from latest to oldest. Each listing includes a | |||
| * `GET /info` now returns `SecurityOptions` field, showing if `apparmor`, `seccomp`, or `selinux` is supported. | |||
| * `GET /networks` now supports filtering by `label` and `driver`. | |||
| * `POST /containers/create` now takes `MaximumIOps` and `MaximumIOBps` fields. Windows daemon only. | |||
| * If no command specified when create a container, an HTTP 400 is now returned instead of a 500. | |||
There was a problem hiding this comment.
Perhaps
* `POST /containers/create` now returns a HTTP 400 "bad parameter" message
if no command is specified (instead of a HTTP 500 "server error")
|
Thanks @wangxing1517! I left one suggestion, can you update that, and squash your commits? (It's ok to have just a single commit for this PR) |
GordonTheTurtle
added
the
dco/no
|
Oh, looks like you introduced a merge-commit in that last push; can you try doing a rebase and squash your commits? You can also find some instructions in our contributors guide; https://docs.docker.com/opensource/workflow/work-issue/#pull-and-rebase-frequently |
Signed-off-by: Wang Xing <hzwangxing@corp.netease.com>
coldTea214
force-pushed
the
fix_mistake_400_for_500
branch
from
3a071fd
to
beca261
Compare
GordonTheTurtle
removed
the
dco/no
|
Thanks for your kind reminder. |
|
LGTM, thanks! ping @MHBauer @coolljt0725 PTAL |
|
@wangxing1517 thank you |
|
hmf, WindowsTP5 is running into a flaky test; #22601. Restarting |
|
LGTM |
cherry-picked from upstream moby#22762 When users run/create a container without specifying a command, daemon will return 500 error instead of 400 error. For example: Dockerfile: From debian:jessie CMD [] docker build -t test . docker create/run test Signed-off-by: Wang Xing hzwangxing@corp.netease.com Signed-off-by: xiekeyang <xiekeyang@huawei.com>
Fix bug which mistakes 400 error for 500 cherry-picked from upstream moby#22762 When users run/create a container without specifying a command, daemon will return 500 error instead of 400 error. For example: Dockerfile: From debian:jessie CMD [] docker build -t test . docker create/run test Signed-off-by: Wang Xing hzwangxing@corp.netease.com Signed-off-by: xiekeyang <xiekeyang@huawei.com> See merge request docker/docker!611
When users run/create a container without specifying a command, daemon will return 500 error instead of 400 error.
For example:
Signed-off-by: Wang Xing hzwangxing@corp.netease.com