-
Notifications
You must be signed in to change notification settings - Fork 9.1k
v3.2: Support ordered multipart including streaming #4589
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
Open
handrews
wants to merge
9
commits into
OAI:v3.2-dev
Choose a base branch
from
handrews:mixed
base: v3.2-dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+191
−16
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
afeedf8
Support ordered multipart including streaming
handrews 1a133aa
Fix formatting in Encoding guidance
handrews 3b92867
Clarify multipart preamble/epilogue
handrews aeddb74
Clarify lack of nested multipart support.
handrews be6ba1d
Fix typo
handrews ccd95a0
Apply suggestions from code review
handrews 20e8042
Fix typo
handrews b7cd335
Better word ordering
handrews 5571d1f
Make Encoding type resolution explicit
handrews File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Pity, my only multipart use case is that the multipart request consists of parts that are either of
application/http
, ormultipart/mixed
with parts of media typeapplication/http
,see Multipart Batch Format.
I had assumed I can model that with an
itemSchema
that isoneOf
an array of HTTP requests or a single HTTP request, but the corresponding Encoding Object may be difficult.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.
@ralfhandl It's telling that the two people to give the most comprehensive reviews of this PR to date both need nested
multipart
. I do have thoughts on how to do this, and would be more than happy to consider it part of this release-blocking functionality for 3.2. It would involve addingencoding
,prefixEncoding
, anditemEncoding
to the Encoding Object, but there are challenging edge cases and I don't want to drag this PR down with them. I'd rather get this merged, submit a follow-on PR (which is impractical without merging this first), and then we can decide if the follow-on works.Regarding
application/http
, the problem (to me) is not "how to use an Encoding Object withapplication/http
" but "how to modelapplication/http
at all." Can you point me to how you would model anapplication/http
payload if it were not in amultipart
part? I realize that would be a rather strange payload outside ofmultipart
, but please humor me- feel free to open a new discussion on it if there's no clear answer. If we know how to model it in general, we can look at how to model it as amultipart
part. Although that might have to wait for 3.3.