-
Notifications
You must be signed in to change notification settings - Fork 21
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
Version is missing in the response headers #70
Comments
Hey there @nicolaa! I'm working on this issue now (with a mentor through a program called RubyMe). I'll be sending a PR very soon! :-) |
In issue stitchfix#70 we see that response headers do not respond with the correct version. This adds a rack middleware that overrides the Content-Type header to show the version. Co-authored-by: Juan Carlos Ruiz<[email protected]>
Hi @nicolaa - can you say a bit more about why this is a problem? |
@brettfishman This actually came up when one of our service consumers wrote pact expectations for header values. They were calling the v2 endpoint and specifying |
@nicolaa Ah, got it. Thanks for adding the additional context. I agree that it seems appropriate for the server to acknowledge the UPDATE: In light of section 3.1.1.5 in the RFC for |
@nicolaa So after thinking on this a bit more, and conferring with @bwebster on it, I'm not sure we are convinced that this is an issue worth solving at this point for a couple reasons:
Would you be able to point me to a source that supports your suggested approach? |
@brettfishman I think you may be right. I'm not sure the version info should be tied to the Content-Type header either. HOWEVER, we do attach it to both the Content-Type and Accept headers in the Request, and there is no corresponding Accept header in the Response, so it feels like there is still a gap somewhere. I think most REST standards propose a customer header like Accept-Version to handle this. I don't feel super strongly about this either way, but in the example i was referring to, the v1 and v2 response bodies are different, so I do think that there should be some way in the Response to differentiate what "Type" of response body we are sending. |
@nicolaa I agree with this statement. I don't have bandwidth at the moment to research other potential solutions, but would welcome a proposal for how to achieve this. I don't think the current proposal totally works, but I'm happy to revisit my POV if a compelling solution is found. |
@brettfishman TBH, I don't think a perfect solution exists. These are known limitations of HTTP. We are really trying to build on top of an infrastructure that wasn't designed for our needs. |
@nicolaa yeah, agreed. To be clear, I'm definitely not seeking a perfect solution. |
Hey there! I appreciate the consideration of this pull request!
…-Alicia
On Thu, Jun 13, 2019 at 4:38 PM Brett Fishman ***@***.***> wrote:
@nicolaa <https://github.com/nicolaa> yeah, agreed. To be clear, I'm
definitely not seeking a perfect solution.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#70?email_source=notifications&email_token=ADJTK2KGOWO4QOUK6SGEVSTP2KV5LA5CNFSM4HDEYG6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXU623I#issuecomment-501869933>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADJTK2NCQKCH7P4ERTISBQDP2KV5LANCNFSM4HDEYG6A>
.
|
Problem:
ApiVersionConstraint
requires clients to sendAccept
request headers that specify the API version, for example:Accept: "application/json; version=2"
. However, the API response does not match this expected behavior.Solution:
Add a default header to
Api::ApiController
that responds with the correct version.For example:
Content-Type: "application/json; version=2"
The text was updated successfully, but these errors were encountered: