-
Notifications
You must be signed in to change notification settings - Fork 16
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
Rename Content-*
→ Patch-*
within Patches
#120
Conversation
Discussion in Meeting-67:
|
Chair summary: The discussion above (from Meeting-67) is mostly about framing rather than header names, which we should discuss in a separate issue and PR. On the other hand, @CxRes points out that if we do switch to multipart framing, and want to obey the MIME spec strictly, we might be forced to use header names that are prefixed with
We had further elaboration about the MIME spec on Discord:
So as we evaluate naming in this draft, we can keep in mind that if we later want to adopt MIME multipart framing, we might (or might not) be restricted to using |
RFC2046 -> 5.1. Multipart Media Type Page 17
This is not a real impediment, even if you have to follow the standard strictly, provided you accept the following trick. You can leave the header of the part (of the multipart) empty (or with any |
I'd not be happy about this trick, as this approach is similar to the one that is currently used (although without multipart/* formatting), but it requires some special processing of message bodies (to assume that they may start with headers), which makes it difficult to work with existing web frameworks and tools. |
@@ -358,7 +358,7 @@ Table of Contents | |||
Patches: OK | |||
|
|||
|
|||
Every patch MUST include either a Content-Type or a Content-Range. | |||
Every patch MUST include either a Patch-Type or a Content-Range. |
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 like I missed a Content-Range
->Patch-Range
here. This one should be changed, too.
Content-*
→ Patch-*
Content-*
→ Patch-*
Content-*
→ Patch-*
within Patches
After some reflection, I want to withdraw this proposal. I'll explain why here. I'm seeing a deeper meaning to why everyone is talking about framing here— this PR is subtly (and confusingly) trying to solve a framing issue by making a naming change. Recall that the motivation for this PR is that The framing issue we're trying to solve is just a perceptual aesthetic— when an update uses a
However, having multiple Patches per Update is actually the rare case. The common case will be a single Patch per Update, like this:
It's not as big a deal here. And unfortunately, this PR has the cost of making header names inconsistent in the rare case, by renaming all Content-Range and Content-Length headers to Patch-Range and Patch-Length, just because they appear within a multi-patch update. I think this change is not worth the cost. Consider that Patches can appear in many places in HTTP messages:
It would be very nice to have a uniform syntax for patches across all situations! But this PR introduces an inconsistency—any patch within a I don't think we should be solving one confusing thing by introducing another confusion. If we want to make framing more legible, I think we should change framing directly. So I offer to withdraw this proposal to rename Content-Range and Content-Length within Patches: N. On the other hand, for the specific issue of specifying the type of a patch, I think it does makes sense to use Patch-Type instead of Content-Type. I'll submit a separate PR for that. |
(Chair:) Nobody has objected. Withdrawing this PR. |
Draft Renaming Scheme for Issue #97
Synopsis of the issue:
Content-Length
,Content-Type
, andContent-Range
headers to describe patchesPatch-Length
,Patch-Type
, andPatch-Range
in subscriptionsPatches: 1
is equivalent to a top-level HTTP patchThis PR proposes a naming rule, illustrates it with examples, and provides edits to the spec to support it.
The Proposed Rule
Updates expressed at the top level of PUT and GET use existing HTTP headers (except Patch-Type, which is new):
Updates expressed within a
Patches: N
block use the Patch-* variant:The PATCH method continues to use Content-Type instead of Patch-Type.
Advantages
Patches: N
blockPatches: N
avoids corrupting legacy servers with Partial PUTExamples
Range Patch with PUT at top level
Notes:
Content-*
uniformly.Range Patch with PUT within a
Patches: 1
blockNotes:
Patch-*
uniformly.Custom Patch Type at top-level using
PATCH
Notes:
Custom Patch Type with PUT at top-level
Note:
Custom Patch Type using PUT within
Patches: 1
:Note:
GET Subscription with
Patches: N
blocksNote:
Patch-*
uniformly for all the patch information.GET Subscription with snapshots
Note:
Content-*
uniformly.GET responding with Custom Patch Type in
Patches: N
blockNote:
Patch-*
uniformly for all the patch information.GET responding with Custom Patch Type at top level
Note: Impossible, because
Content-Type
clobbers.Comparisons with current spec
Range Patch with PUT at top-level
Notes:
Range Patch with PUT within
Patches: 1
blockNotes:
Content-*
as top-level patches in previous example.Custom Patch Type with PUT is Impossible
Note: Impossible, because
Content-Type
clobbers:Custom Patch Type with PATCH
Notes:
Custom Patch Type within a
Patches: 1
blockUses Content-*
Notes:
Thoughts?