Skip to content
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

Convert botocore.awsrequest.AWSResponse to dict #759

Closed
wants to merge 1 commit into from

Conversation

dazza-codes
Copy link
Contributor

@dazza-codes dazza-codes commented Feb 14, 2020

  • it might help if AWSResponse.to_dict() could
    take care of business, but that would be too easy

Possible fix for #755 but it's a guess really and it might be specific to moto.

When this PR code is used against live-s3, it works as expected to list buckets and issue a HEAD request on them, and when a print statement is added, i.e.

  • print(f"http_response is {type(http_response)}")
  • the type reported is:
http_response is <class 'aiobotocore._endpoint_helpers.ClientResponseProxy'>

With moto, the http_response is an botocore.awsrequest.AWSResponse, so this is something different. Maybe something is not registered as usual when using moto vs. live-s3 endpoints? Maybe moto is registering something with the creating-client-class and it mutates the class_attributes or something to replace some aiobotocore attributes? There's a few events it could use to intercept aiobotocore behavior.

The unit tests are passing on this PR with both moto and live-s3 endpoints, but it's not entirely clear whether the response data is already retrieved via aiohttp at the point when the parser is called or whether parsing calls should be able to await a read() of the content.

@codecov
Copy link

codecov bot commented Feb 14, 2020

Codecov Report

Merging #759 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #759      +/-   ##
==========================================
+ Coverage   91.15%   91.16%   +0.01%     
==========================================
  Files          12       12              
  Lines         678      679       +1     
==========================================
+ Hits          618      619       +1     
  Misses         60       60
Impacted Files Coverage Δ
aiobotocore/endpoint.py 89.28% <100%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 680814b...ddc70d9. Read the comment docs.

@thehesiod
Copy link
Collaborator

sorry wasted my whole night trying to fix moto on 3.5, will have to look later

- it might help if AWSResponse.to_dict() could
  take care of business, but that would be too easy
@dazza-codes dazza-codes mentioned this pull request Feb 19, 2020
@dazza-codes
Copy link
Contributor Author

dazza-codes commented Feb 19, 2020

This PR should not be merged. It fixes a problem with moto mock responses by changing how they are parsed. This is not a good fix for that problem. A better fix is to avoid the moto mock responses so that the test code can hit a moto server; this is described in #755 and example fixes are in #766

@dazza-codes
Copy link
Contributor Author

Closing this in favor of #766

@dazza-codes dazza-codes deleted the no-raw-headers branch February 19, 2020 18:00
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.

2 participants