-
Notifications
You must be signed in to change notification settings - Fork 1
fix ResponseMetaDataTransformer #13
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,8 +102,14 @@ def transform(self, input_data: dict, *, ctx: TransformContext) -> dict: | |
if k == "ResponseMetadata": | ||
metadata = v | ||
http_headers = metadata.get("HTTPHeaders") | ||
if not isinstance(http_headers, dict): | ||
continue | ||
|
||
# TODO "x-amz-bucket-region" | ||
# TestS3.test_region_header_exists -> verifies bucket-region | ||
|
||
# FIXME: proper value is `content-type` with no underscore in lowercase, but this will necessitate a | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a big problem actually. It's not the first time that we defer fixing issues in snapshot library because of backwards compatibility. Thanks for highlighting it! I added the issue about this to snapshots library backlog. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does yes. Maybe versioning could actually help here? Store the snapshot library version along the snapshot, to enjoy new features / support backward compatibility. Thanks for adding the issue on the backlog. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, versioning is the option I added to the backlog issue. It is not the straightforward one when projected onto test runs as we will need to run tests with different dependency versions, but definitely something to try. |
||
# refresh of all snapshots | ||
headers_to_collect = ["content_type"] | ||
simplified_headers = {} | ||
for h in headers_to_collect: | ||
|
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.
Good handling of non-expected values here 👏