revert (partial): write compressed configuration [RHELDST-25461] #728
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 commit partially reverts db4a344; specifically, it disables the writing of compressed config and reverts back to writing JSON config as before. This is being reverted due to an issue noticed during testing. It seems that the written items are ending up with *two* layers of base64 encoding, which is not intended. The boto docs[1] use base64 strings as example arguments, giving the impression the caller is expected to take care of base64 encoding, but in fact botocore internally does the encoding; if the client also encodes, we end up with two layers of encoding. There is also a bug filed relating to this[2]. The code here still seems to "work" since the same mistake is made on both the writing and reading end, but the goal is to make the config smaller and having double-encoding works against that. It should be easy enough to fix, but I'd like some time to confirm my understanding of how it works, check whether exodus-lambda needs a fix and also check whether localstack and AWS are behaving the same. Hence I'll revert this for now and keep writing the old style of config. [1] https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/dynamodb/client/put_item.html [2] aws/aws-cli#1097
rohanpm
force-pushed
the
disable-compressed-config
branch
from
005d272 to
4e8cac6
Compare
This error started to appear in github pylint runs:
exodus_gw/models/dramatiq.py:19:8:
E1136: Value 'Mapped' is unsubscriptable (unsubscriptable-object)
I'm not sure why, but the error is certainly wrong. Disable the check in
this file.
crungehottman
approved these changes
Aug 5, 2024
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 commit partially reverts db4a344; specifically, it disables the writing of compressed config and reverts back to writing JSON config as before.
This is being reverted due to an issue noticed during testing. It seems that the written items are ending up with two layers of base64 encoding, which is not intended.
The boto docs[1] use base64 strings as example arguments, giving the impression the caller is expected to take care of base64 encoding, but in fact botocore internally does the encoding; if the client also encodes, we end up with two layers of encoding.
There is also a bug filed relating to this[2].
The code here still seems to "work" since the same mistake is made on both the writing and reading end, but the goal is to make the config smaller and having double-encoding works against that.
It should be easy enough to fix, but I'd like some time to confirm my understanding of how it works, check whether exodus-lambda needs a fix and also check whether localstack and AWS are behaving the same. Hence I'll revert this for now and keep writing the old style of config.
[1] https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/dynamodb/client/put_item.html
[2] aws/aws-cli#1097