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

Add patch support for pg8000 (Pure python driver) #115

Merged
merged 2 commits into from
Dec 26, 2018

Conversation

ridha
Copy link
Contributor

@ridha ridha commented Dec 17, 2018

Issue #, if available:

Description of changes:

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

@@ -39,7 +39,7 @@ def test_execute_dsn_kwargs():
cur = conn.cursor()
cur.execute(q)

subsegment = xray_recorder.current_segment().subsegments[0]
subsegment = xray_recorder.current_segment().subsegments[-1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution. I have one question. This test setup explicitly patches only psycopg2 in line 12. How does this get impacted? You can refer httplib patcher where it has unpatch method so for test teardown it restores the original httplib to avoid impacting other libraries depending on it: https://github.com/aws/aws-xray-sdk-python/blob/master/aws_xray_sdk/ext/httplib/patch.py#L168-L178. It's also required that double instrumentation is not the default for customers running patch_all. You can check the logic here: https://github.com/aws/aws-xray-sdk-python/blob/master/aws_xray_sdk/core/patcher.py#L30-L34

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing! Fixed in the latest revision.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I've merged it and will roll it out in our next release.

@haotianw465 haotianw465 merged commit 3bc68cf into aws:master Dec 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants