-
Notifications
You must be signed in to change notification settings - Fork 66
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
Add RFC 006: Format Negotiation Protocol #115
base: master
Are you sure you want to change the base?
Conversation
docs/rfc/006-format-negotiation.md
Outdated
### Definitions | ||
|
||
- A *version* is a string of the form `x.y` where x and y are positive integers | ||
- A *Conjure format identifier* is a string of the `application/<format>+conjure; v=<version>` where `<format>` is a |
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.
Based on rfc6838 section 4.2.8 I think the <format>
and conjure
strings are inverted, however I would prefer that we do not invent new media types, but rather use existing well-supported types with parameters to convey conjure metadata. <media type>; conjure=<version>
where we constrain that the type
portion of the media type (see rfc2616 section 3.7) must be application
.
Existing implementations of the proposed +
delimited type map more closely to individual conjure apis, for which a conjure file is the specification, for example application/timelock+json
.
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.
you're right, thanks. I'd be happy with application/json; conjure=1.0
and application/cbor; conjure=2.0
, etc.
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.
Please could we actually just use a single integer instead of the x.y
thing? I don't want to end up having version matchers that have to figure out is this a 1.4 request or a 1.3 request...
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.
can do
docs/rfc/006-format-negotiation.md
Outdated
|
||
### Definitions | ||
|
||
- A *version* is a string of the form `x.y` where x and y are positive integers |
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.
Should we decouple the specifications for conjure versioning from media types? They touch a similar space, but I don't think they're directly related.
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.
what do you propose?
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.
I agree with the content. It may be simpler to spec out content types without a version separately, then add a specification for versioning as RFC 007. No strong preference, feel free to disregard.
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.
think i'll keep it in here.
docs/rfc/006-format-negotiation.md
Outdated
|
||
At the beginning of a session, clients have no knowledge of the list of formats supported by the server. Clients may | ||
update the list of formats supported by a given server or service as per the formats advertised in the Accept headers of | ||
responses in the session. To bootstrap the negotiation, client shall assume that servers support a "recent" variant of |
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.
Can you clarify on the "recent" variant of the application/json+conjure format
and what it means to have formats fall into the same family?
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.
will clarify
docs/rfc/006-format-negotiation.md
Outdated
Accept response header. (Note that this is a non-standard header for HTTP 1.1 responses.) | ||
|
||
**Negotiation.** | ||
Format negotiation takes place in the context of a "session" of subsequent requests made by a client to a server (or |
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.
when do we consider a client-server negotiation is done? and when do client and server need to renegotiate the content format?
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.
will clarify
- A *version* is a string of the form `x.y` where x and y are positive integers | ||
- A *Conjure format identifier* is a string of the `application/<format>+conjure; v=<version>` where `<format>` is a | ||
non-empty string over `[a-z]` and `<version>` is a version string (as above) | ||
- A *Conjure format list* is a comma-separated, ordered list of Conjure format identifiers |
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.
should we use q
(quality factory), according to RFC2616, instead?
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.
Don't think the version is related to quality?
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.
As using it to describe the priority order of the conjure format identifiers instead of assuming that this list is ordered, which the spec doesn't explicitly call out?
But according to this, order of the list doesn't seem to matter if no q
being defined.
If there is no priority defined for the first two values, the order in the list is irrelevant.
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.
I don't think I've seen the quality parameter used in practice, most implementations I've worked with use order.
If the quality of types matches we would use order to break ties. Given that, I don't see any benefit that the quality parameter provides.
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.
Is it technically harder to support the quality factor? If not, then regardless of other people's use of it I would argue it's probably beneficial to stick to the advertised standard, even if not just for discoverability (e.g. 'search-engine of choice'ing Accept header format
will get you a generic description from Mozilla which matches our use, rather than something which is subtly different, being close enough you will think you know how it works, but actually don't).
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.
it means we'd eat an extra parse and sort pass for each request. probably doesn't matter much.
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.
You'd just cache results; doesn't matter at all :)
docs/rfc/006-format-negotiation.md
Outdated
header. The format indicates the preference-ordered list of formats that the client supports. | ||
|
||
**Responses.** | ||
Servers that do not support the request format respond with a suitable error code. Otherwise, they use the |
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.
suitable error code
I'd prefer we define what this error code is.
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.
Two options:
10.4.7 406 Not Acceptable
The resource identified by the request is only capable of generating response entities which have content characteristics not acceptable according to the accept headers sent in the request.
Unless it was a HEAD request, the response SHOULD include an entity containing a list of available entity characteristics and location(s) from which the user or user agent can choose the one most appropriate. The entity format is specified by the media type given in the Content-Type header field. Depending upon the format and the capabilities of the user agent, selection of the most appropriate choice MAY be performed automatically. However, this specification does not define any standard for such automatic selection.
Note: HTTP/1.1 servers are allowed to return responses which are
not acceptable according to the accept headers sent in the
request. In some cases, this may even be preferable to sending a
406 response. User agents are encouraged to inspect the headers of
an incoming response to determine if it is acceptable.
If the response could be unacceptable, a user agent SHOULD temporarily stop receipt of more data and query the user for a decision on further actions.
10.4.16 415 Unsupported Media Type
The server is refusing to service the request because the entity of the request is in a format not supported by the requested resource for the requested method.
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.
415 seems closest, but don't feel strongly
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.
fixed
think i touched on all comments, take another look please. |
I think @iamdanfox had some high-level concerns with this, will try to clear offline. |
docs/rfc/006-format-negotiation.md
Outdated
non-empty string over `[a-z]` and `<version>` is a version string (as above) | ||
- A *version* is a non-zero integer | ||
- A *Conjure format identifier* is a string of the `application/<format>; v=<version>` where `<format>` is a | ||
non-empty string over `[a-z]` (e.g., `json`) and `<version>` is a version string (as above) |
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.
I think we should include hyphen in the format specification. application/x-jackson-smile
for example.
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.
done
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.
Thanks!
docs/rfc/006-format-negotiation.md
Outdated
### Definitions | ||
|
||
- A *version* is a non-zero integer | ||
- A *Conjure format identifier* is a string of the `application/<format>; v=<version>` where `<format>` is a |
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 wondering if we could maybe use "conjure_version" or "conjure_protocol" instead of just "v"? Or is this a standardized version parameter (I didn't read the standard). It could be more discoverable that way if someone bumps into this in the wild.
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.
meant to change this to conjure=<version>
as per above comment. pushed now.
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.
Did you push? I don't see the changes
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.
now I did
docs/rfc/006-format-negotiation.md
Outdated
### Wire format versioning | ||
|
||
We propose that every revision of the Conjure wire format be labelled with a format identifier. | ||
The current PLAIN+JSON format shall be labelled `application/json; v=1`. |
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.
What exactly is this versioning? JSON bit is covered by application/json, so really this is kinda conjure DSL (IR?) format version, not "PLAIN+JSON" format.
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.
Not sure if PLAIN should be considered part of the format here.
It only refers to the encoding of parameters NOT in the body (headers, query and path params), things that are orthogonal to whatever the content-type
is.
@jkozlowski I think it's fine to say it's application/json
since every response is JSON compatible.
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.
I think it should version the PLAIN and the structured format together.
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.
I guess we do want that. It just seemed weird to requisition content-type
for the format of things outside the body, but can't think of a better place that would explicitly cover the format of custom headers etc.
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.
yep
docs/rfc/006-format-negotiation.md
Outdated
|
||
**Responses.** | ||
Servers that do not support the request format respond with Conjure error UNSUPPORTED/415. Otherwise, if the server | ||
does supported the request format, it uses the most-preferred (as per Accept request header) format to encode the |
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.
nit: "does support the"
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.
fixed
docs/rfc/006-format-negotiation.md
Outdated
**Responses.** | ||
Servers that do not support the request format respond with Conjure error UNSUPPORTED/415. Otherwise, if the server | ||
does supported the request format, it uses the most-preferred (as per Accept request header) format to encode the | ||
response and advertise the chosen format in the response Content-Type header. |
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.
So this means I can send "application/json" in my first request, but actually put CBOR as my preference, so I'll get CBOR request? Neat, and then I can followup all my other ones with CBOR
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.
... you'd get a CBOR response, yes.
docs/rfc/006-format-negotiation.md
Outdated
### Wire format versioning | ||
|
||
We propose that every revision of the Conjure wire format be labelled with a format identifier. | ||
The current PLAIN+JSON format shall be labelled `application/json; v=1`. |
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.
We should also probably just put down that "application/json" is also fine for backcompat.
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.
added.
docs/rfc/006-format-negotiation.md
Outdated
service). A client may assume, loosely, that a session is active to a given server or service as long as it receives | ||
responses with non-error responses. | ||
|
||
At the beginning of a session, clients have no knowledge of the list of formats supported by the server. Clients may |
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.
Is this negotiation done per IP? So if I have many nodes of my service, each of them could potentially be talking different wire formats?
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.
What about the case of a blue/green deploy behind a proxy? Does Conjure version become part of rolling-upgrade state (as in, the server cannot advertise the new conjure protocol until all nodes have upgraded)?
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.
I don't think we need to explicitly handle this complexity. Clients receiving a 415 error can choose to retry the request with a more compatible format.
docs/rfc/006-format-negotiation.md
Outdated
update the list of formats supported by a given server or service as per the formats advertised in the Accept headers of | ||
responses in the session. Typically, a client will pick its most preferred format that is advertised by the server | ||
in the previous response of the session. To bootstrap the negotiation, clients shall assume that servers support the | ||
most recent variant of the `application/json` format known to the client, and use this version for the first request of |
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.
I guess that works, but maybe it's not so bad to always send application/json; v=1 on the first request than to fail the request because you jumped the gun on upgrading your client before everyone else.
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.
I think both are OK.
docs/rfc/006-format-negotiation.md
Outdated
response and advertise the chosen format in the response Content-Type header. | ||
|
||
Every response (including non-success responses) must send a preference-ordered format list of supported formats as | ||
Accept response header. (Note that this is a non-standard header for HTTP 1.1 responses.) |
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.
It may be worth switching from non-standard use of the Accept
header to using the proposed Accept-Post
response header (https://tools.ietf.org/id/draft-wilde-accept-post-00.html).
There is also some question around including the servers Accept
ed types on every response which I think encourages negotiation to potentially give rise to requests and responses both using different formats. If the response is not a 415 response then the response Content-Type
be taken as the agreed upon type for all future communication.
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.
+1 for Accept-Post
rather than misusing Accept
.
We could leave this part off in favor of using the response Content-Type
for simplicity for the time being. Failure case is a service which only returns binary
or no content, preventing the client from upgrading to a preferred media type.
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.
The draft seems abandoned (GitHub repo dead), or at least it never made it into an RFC. So I think 'Accept' and 'Accept-Post' are equally 'non-standard'.
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.
Yeah, I think letting the server choose the format based on the preference list sent by the client should be roughly equivalent. Thoughts on that simpler protocol, @markelliot ?
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.
Fine with me; I think the server returning what it supports is a nice-to-have. If we find a lot of problems in this space. There's an interesting thing here around what content type should be used for errors, I'd propose all errors are sent as JSON format, and thus we can spec the details of server support into the error body as Conjure error params, if we want.
docs/rfc/006-format-negotiation.md
Outdated
``` | ||
|
||
In the rare case that server does not support the bootstrap format, the error response will carry a list of supported | ||
formats (see above) from which the client can choose when retrying the request. |
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.
If we think errors will indeed be rare then we could always fallback to use of an additional OPTIONS
request to find out what the acceptable kinds of Content-Type
are if we wanted to adhere to HTTP/1.1
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.
yep, that's also an option
Hey - the W3C (or maybe the IETF) standards have a requirement of a reference implementation to prove the RFC makes sense. For something like this, I'd love to see such a reference implementation! |
We have the following requirements for the format negotiation protocol: | ||
- The protocol should in almost all cases not incur additional round-trips | ||
- Clients and servers will eventually agree on the protocol that is (1) most preferred by the client and (2) | ||
supported by the server. That, clients drive the negotiation, constrained by server capabilities. |
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.
Grammatically weird? That, clients drive the negotiation, constrained [...]
|
||
- A *version* is a positive integer | ||
- A *Conjure format identifier* is a string of the `application/<format>; conjure=<version>` where `<format>` is a | ||
non-empty string over `[a-z-]` (e.g., `json`) and `<version>` is a version string (as above) |
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.
probably want this to be [a-z][a-z-]
instead?
Looks like this RFC is stuck in between alternatives. I'll schedule an in-person review to hash things out. |
Posted on RFC tracker, please join if you like. |
docs/rfc/006-format-negotiation.md
Outdated
most recent variant of the `application/json` format known to the client, and use this version for the first request of | ||
every session. | ||
|
||
**Example.** The following sequence of two requests and corresponding responses are between a client that supports CBOR |
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.
Please could we break out this single "example" heading into a few separate standalone examples that each illustrate parts of the behaviour this RFC describes, e.g.
- example 1: old client talking to new server (successfully upgrade to json2)
- example 2: new client talking to old server (stay on lowest common denominator)
- example 3: new client talking to old server (starts with a new protocol, gets an immediate 415, client retries with a more conservative request)
- example 4: new client talking to old server from before this RFC (i.e. just complies with the current wire spec)
Also I think some of the examples should illustrate what happens when the first endpoint is a Conjure binary
endpoint, i.e. immediately sends a application/octet-stream
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.
done
@cakofony @iamdanfox @j-baker updated as per discussion last week. thanks for your patience. |
@cakofony has an in-progress PR to implement a PoC implementation |
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.
Looks good to me, thank you @uschi2000
The current proposal seems to rely a lot on preexisting shared knowledge between the client and server of what formats are "widely supported". If this is going to be the case I think we should have a much stricter definition of what formats will be more supported than others. This has to place restrictions on future conjure implementations though since we have to define now the relationships between future versions (such as always supporting json, or some such). It was my understanding that one of the purposes of this RFC is to explicitly avoid that. There were previously ideas around the server communicating something back to the client and I think specifically in the case of 415s this makes sense. There is nothing preventing the 415 from including a simple response body (the actual content does not matter and can be assumed to be ignorable) with the most preferred format from the If clients support a reasonable number of formats this becomes critical as it bounds the maximum number of requests needed to a fixed number, rather than the number of formats. Example: Client and server do not share a format - Client supports cbor and json for conjure versions 1 and 2, server does not support any of these (e.g. on conjure=3 which is totally backwards incompatible) Before
After
Example: Clients default request type not supported - Client supports json, cbor, yaml, conjure v 3, 4, 5. Server supports yaml, conjure v 4 Before
After
|
@cbrockington the client can choose to include all client-supported formats in the (first or subsequent) request. this guarantees that the request succeeds iff there exists a format supported by server and client. |
Of course, and it's done so in my examples, though the Unless |
Ah, I see. We could add the Content-Type to the response, of course. It's a bit odd because the body wouldn't actually contain any payload... |
We would probably need some content as to ensure there are no unexpected deserialization errors, but I think the restriction in the RFC need be no more than "something deserializable by that format" (an empty body may fulfill this). For our currently supported formats this could just be some null representation, or a string of the format, error message or some such. Again the actual content would be unimportant. |
I'd prefer not to add a It is an optimization on the "Cutting edge client" example (eager client). I don't think the proposed content-type on non-service-error response is the best path forward, compared to other alternatives that have been suggested ( |
As I said, there is no reason for the content to be empty, it can and should be something which deserializes with that content type (which depending on the format, could be empty). If it's decided it lies outside the scope of this RFC that's fine, but I think we should change the suggestive language that on error clients try to use "a more widely-supported format" to simply trying "a different format", since we are providing no guarantees around which formats are more supported. |
changed the language accordingly, @cbrockington |
docs/rfc/006-format-negotiation.md
Outdated
@@ -73,7 +73,9 @@ is not versioned. | |||
protocols based on their preference and let the server merely "chooses" based on its support for the most preferred | |||
formats. Further, clients are in control of the trade-off between choosing the newest or most preferred versus an older | |||
or more widely supported format. The former approach unlocks new formats and features more quickly, but may result in an | |||
additional round-trip when the client has to reissue the request encoded with a more widely supported format. | |||
additional round-trip when the client has to reissue the request encoded with a different und hopefully supported |
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.
*and
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.
moving to munich has already corrupted my typing ...
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.
fixed
This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days. |
Rendered