Skip to content

Fix patching for PynamoDB 4.x with botocore 1.13 #181

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

Merged
merged 2 commits into from
Nov 19, 2019
Merged

Fix patching for PynamoDB 4.x with botocore 1.13 #181

merged 2 commits into from
Nov 19, 2019

Conversation

Dunedan
Copy link
Contributor

@Dunedan Dunedan commented Nov 11, 2019

Fixes #179

Patching PynamoDB broke when using botocore 1.13, as this version of botocore doesn't ship requests anymore. While the vendored requests library is a requirement for PynamoDB <4.0, it's not for PynamoDB >=4.0. Therefore only import and patch botocore.vendored.requests.sessions when using PynamoDB <4.0.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@yan12125
Copy link
Contributor

Ah, I happen to work on the same issue :) Maybe _xray_enabled should be added to botocore.httpsession for PynamoDB 4, like https://github.com/yan12125/aws-xray-sdk-python/commit/cc697afe4c18f15622c0348df5e0c3a383a9eecd?

There are still test failures. The reason might be incompatibility between aiobotocore and botocore >= 1.13 aio-libs/aiobotocore#743.

@Dunedan
Copy link
Contributor Author

Dunedan commented Nov 11, 2019

Maybe _xray_enabled should be added to botocore.httpsession for PynamoDB 4

I thought about that as well, but assumed there was a reason it wasn't added during the PynamoDB 4 commit. Anyway it seems to make sense, so I added it now.

There are still test failures. The reason might be incompatibility between aiobotocore and botocore >= 1.13 aio-libs/aiobotocore#743.

I focused on getting the PynamoDB patching to work again with botocore 1.13. I haven't looked too deep into the aiobotocore failures, but to me it seems like this doesn't require a change to aws-xray-sdk-python, but just a new version of aiobotocore.

@polamayster
Copy link
Contributor

polamayster commented Nov 11, 2019

Just curious, what about pynamodb versions < 4.0 but with botocore >= 1.13 and removed vendored requests, wouldn't it fail with this fix?

@bhautikpip
Copy link
Contributor

bhautikpip commented Nov 11, 2019

Hi @Dunedan,

Thanks for submitting this PR. Looks like few tests have failed because aiobotocore version is still making dependency on botocore.vendored.requests package. This does not require any change in aws-xray-python-sdk. Hopefully upcoming version of aiobotocore removes this dependency.

@Dunedan
Copy link
Contributor Author

Dunedan commented Nov 12, 2019

@polamayster I believe PynamoDB < 4.0 is simply not compatible with botocore >= 1.13, as the dependency on botocore.vendored.requests got just removed in PynamoDB 4.0.

@yan12125
Copy link
Contributor

yan12125 commented Nov 12, 2019

Now aiobotocore 0.11 is out with compatibility with new botocore, and aws-xray-sdk-python tests are green with that.

@codecov-io
Copy link

codecov-io commented Nov 19, 2019

Codecov Report

Merging #181 into master will decrease coverage by 0.04%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #181      +/-   ##
=========================================
- Coverage   83.44%   83.4%   -0.05%     
=========================================
  Files          77      77              
  Lines        2905    2910       +5     
=========================================
+ Hits         2424    2427       +3     
- Misses        481     483       +2
Impacted Files Coverage Δ
aws_xray_sdk/ext/pynamodb/patch.py 76.08% <44.44%> (-6.84%) ⬇️
aws_xray_sdk/core/sampling/local/reservoir.py 100% <0%> (+11.11%) ⬆️

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 dbdeab6...f1f8cd4. Read the comment docs.

@chanchiem chanchiem self-requested a review November 19, 2019 19:12
@chanchiem
Copy link
Contributor

Thank you very much for your contribution! This looks like a great fix :)

@chanchiem chanchiem merged commit 1d92820 into aws:master Nov 19, 2019
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.

patch(("pynamodb",)) not working with botocore 1.13
6 participants