Support base64 encoding of bytes input for JSON blob types. #397
Conversation
This file contains 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
This pull request modifies the code added in ffcef5f to also support arguments passed in as bytes, such as those you might encounter when using KMS. See this related issue: boto/boto3#31 Since the JSON data for serialization tests doesn't support specifying bytes as input, I've added a new test class to cover this case.
| @@ -338,8 +338,9 @@ def _serialize_type_timestamp(self, serialized, value, shape, key): | |||
| serialized[key] = self._convert_timestamp_to_str(value) | |||
|
|
|||
| def _serialize_type_blob(self, serialized, value, shape, key): | |||
There was a problem hiding this comment.
We are now no longer consistently handling blob types across protocols, as this logic is only in the JSONSerializer, and this is now duplicated from the query protocol. I think we should refactor this so that all the protocols can share a similar blob serializer. There's even a TODO note in the RestXML serializer specifically about this point.
|
+1 to handling this case, but we should port this over to all protocols so they all have the same behavior regarding bytes types. |
This refactors the base64-encoding into a common method shared by all serializers. The serializers now get the value using this new method and just need to set it somewhere in the params.
|
I've updated / refactored the code such that all serializers now use the same shared logic to convert a value to base64. Please take another look @jamesls! |
|
|
|
LGTM 🚢 |
danielgtaylor
added a commit
that referenced
this pull request
Dec 5, 2014
Support base64 encoding of bytes input for JSON blob types.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This pull request modifies the code added in ffcef5f to also support
arguments passed in as bytes, such as those you might encounter when
using KMS.
See this related issue: boto/boto3#31
Since the JSON data for serialization tests doesn't support specifying
bytes as input, I've added a new test class to cover this case.
cc @jamesls, @kyleknap