Skip to content

Skip body encoding when payload requires a binary#62

Merged
philss merged 4 commits intomasterfrom
ps-skip-body-encoding-for-some-operations
Dec 16, 2020
Merged

Skip body encoding when payload requires a binary#62
philss merged 4 commits intomasterfrom
ps-skip-body-encoding-for-some-operations

Conversation

@philss
Copy link
Copy Markdown
Contributor

@philss philss commented Dec 15, 2020

This is necessary to enable some operations like S3's "put object" to
send the payload as a binary (a string or an binary representing a
blob).

A fix to Erlang REST template is coming soon.

This is necessary to enable some operations like S3's "put object" to
send the payload as a binary (a string or an binary representing a
blob).

This closes aws-beam/aws-elixir#41

A fix to Erlang REST template is coming soon.
philss added a commit to aws-beam/aws-elixir that referenced this pull request Dec 15, 2020
This is useful for services that work around files, like S3 or
MediaStoreData. They require use to send the body as it is, without
encoding (they are binaries).

This is related to aws-beam/aws-codegen#62

This closes #41
philss added a commit to aws-beam/aws-elixir that referenced this pull request Dec 15, 2020
This is useful for services that work with files, like S3 or
MediaStoreData. They require us to send the body as it is, without
encoding (they are binaries).

This is related to aws-beam/aws-codegen#62

Closes #41
@philss philss requested a review from josevalim December 15, 2020 03:06
philss added a commit to aws-beam/aws-elixir that referenced this pull request Dec 15, 2020
This is useful for services that work with files, like S3 or
MediaStoreData. They require us to send the body as it is, without
encoding (they are binaries).

This is related to aws-beam/aws-codegen#62

Closes #41
@jfacorro
Copy link
Copy Markdown
Member

Is this related to issue #30?

@philss
Copy link
Copy Markdown
Contributor Author

philss commented Dec 15, 2020

@jfacorro yes! I didn't see that issue 😬 Thanks for pointing out!
I think this fix covers all the services that enable the body as binary.

philss added a commit to aws-beam/aws-erlang that referenced this pull request Dec 15, 2020
This is useful for services that work with files, like S3 or
MediaStoreData. They require us to send the body as it is, without
encoding (they are binaries).

This is related to aws-beam/aws-codegen#62
philss added a commit to aws-beam/aws-erlang that referenced this pull request Dec 16, 2020
This is useful for services that work with files, like S3 or
MediaStoreData. They require us to send the body as it is, without
encoding (they are binaries).

This is related to aws-beam/aws-codegen#62
@philss philss merged commit a5af8e4 into master Dec 16, 2020
@philss philss deleted the ps-skip-body-encoding-for-some-operations branch December 16, 2020 15:05
@robertoaloi
Copy link
Copy Markdown
Member

Hi @philss

I'm getting an error in the Erlang library when trying to fetch from S3, since the client is trying to decode the non-decoded body of the response. I wonder if this is an issue of the Erlang generated module or if you have the same for Elixir. Would it be safe to assume that whenever the "send-body-as-binary" flag is set, the inbound/outbound procedure is the same?

@robertoaloi
Copy link
Copy Markdown
Member

And looking at the actual code, it looks like we need to the same for the output/shape, cause this PR only addresses it for input/shape.

@philss
Copy link
Copy Markdown
Contributor Author

philss commented Mar 22, 2021

Would it be safe to assume that whenever the "send-body-as-binary" flag is set, the inbound/outbound procedure is the same?

@robertoaloi I think there is a way to read that from the output shape. I will try to play with this.
Also, I think this is a problem both for Erlang and Elixir.

In any case, can you open a new issue reporting the problem? Thanks!

@robertoaloi
Copy link
Copy Markdown
Member

I think there is a way to read that from the output shape. I will try to play with this.

Yes, exactly what I found (see my second comment above). Opening a PR, at least for the Erlang part of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants