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

Make version check return 400 instead of 404 #13324

Merged
merged 1 commit into from May 20, 2015
Merged

Conversation

duglin
Copy link
Contributor

@duglin duglin commented May 19, 2015

Because 404 is just wrong

Closes: #13321

Signed-off-by: Doug Davis dug@us.ibm.com

@estesp
Copy link
Contributor

estesp commented May 19, 2015

This looks right-- we don't talk about version mismatch in the docs, so it is not defined right now, but 400 makes way more sense than 404. Do we need a note on version mismatch in the API docs?

code: LGTM

@duglin
Copy link
Contributor Author

duglin commented May 19, 2015

docs update wouldn't hurt. Let me see if I can find a good spot for that.

@thaJeztah
Copy link
Member

@duglin also needs a note in the "what's new" section of the API

@runcom
Copy link
Member

runcom commented May 19, 2015

+1 and code LGTM

@thaJeztah
Copy link
Member

thinking of that (and now it becomes complicated); is this to be considered a "breaking change"? If so; should this only be changed for API >= 1.19?

@duglin
Copy link
Contributor Author

duglin commented May 19, 2015

@thaJeztah since we're talking about an existing error condition here, and just producing a more appropriate message, I'm leaning towards thinking we don't need to worry about breaking people. But if other maintainers feel strongly about it I can add the if-stmt.

Closes: moby#13321

Signed-off-by: Doug Davis <dug@us.ibm.com>
@duglin
Copy link
Contributor Author

duglin commented May 19, 2015

ok - added some doc updates.

@duglin
Copy link
Contributor Author

duglin commented May 19, 2015

ping @LK4D4

@LK4D4
Copy link
Contributor

LK4D4 commented May 20, 2015

LGTM

LK4D4 added a commit that referenced this pull request May 20, 2015
Make version check return 400 instead of 404
@LK4D4 LK4D4 merged commit 53a7953 into moby:master May 20, 2015
@thaJeztah
Copy link
Member

@LK4D4 docs review? 😉

(No worries, I think it's fine :))

@duglin
Copy link
Contributor Author

duglin commented May 20, 2015

ping @moxiegirl @fredlf if you guys see anything wrong with the docs please let me know and I'll open a new PR to fix it.

@moxiegirl
Copy link
Contributor

@duglin Looks good to me Doug.

@duglin
Copy link
Contributor Author

duglin commented May 21, 2015

@moxiegirl great- thanks for checking

@duglin duglin deleted the BadRCOnVersion branch July 10, 2015 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Client API vs Server API version mismatch shouldn't return 404
7 participants