-
Notifications
You must be signed in to change notification settings - Fork 193
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
Fix event stream :content-type
for struct messages
#3603
Conversation
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
fun eventStreamMemberContentType( | ||
model: Model, | ||
memberShape: MemberShape, | ||
protocolContentType: String?, |
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: Just wondering, why do we even allow this to be a nullable string
? In the end, we are going to through an exception in any case.
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.
Some protocols don't support event streams, for example, awsQuery/ec2Query. There isn't a sane value to set here for those.
CHANGELOG.next.toml
Outdated
|
||
|
||
[[smithy-rs]] | ||
message = "Fix event stream `:content-type` message headers for struct messages." |
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: It would be nice if we could also write something that clarifies that the initial content-type for event stream messages differs from the content type for each frame.
You may want to target the release branch with this PR |
A new generated diff is ready to view.
A new doc preview is ready to view. |
Event stream operations with struct shaped messages were using the wrong `:content-type` message header value, which I think wasn't caught before since the supported AWS S3/Transcribe event stream operations don't serialize struct messages. This PR fixes the message content type serialization. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
Event stream operations with struct shaped messages were using the wrong
:content-type
message header value, which I think wasn't caught before since the supported AWS S3/Transcribe event stream operations don't serialize struct messages. This PR fixes the message content type serialization.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.