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 support for Django 2.0 to 3.0 #206

Merged
merged 10 commits into from
Mar 6, 2020
Merged

Conversation

kmichel2
Copy link
Contributor

This fixes a runtime issue with template names in subsegments (#168) and two failed imports during tests.

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

Kevin Michel added 3 commits February 26, 2020 09:43
The previous location, django.core.urlresolvers, was deprecated in 1.10 and removed in 2.0
This middleware was redundant in Django 1.10 and removed in Django 2.0
Django sometimes uses SafeString instead of str for template names.

Jsonpickle serializes SafeString as `{ "py/newargs": ["django/forms/widgets/attrs.html"]}`
instead of `"django/forms/widgets/attrs.html" which is the reason for
the invalid subsegment name.
@kmichel2
Copy link
Contributor Author

Hi,

I fixed the issues I had when deploying xray with Django 3.0 but removed the commit in this PR enabling coverage with tox: I'm too confused about tox to create a correct configuration and only noticed #200 after pushing this code.

@srprash
Copy link
Contributor

srprash commented Feb 26, 2020

Hi @kmichel-sereema
Thanks for the PR. Your tests for Python 3.6+ are failing because of a change in aiobotocore 0.12.0 a couple of days back where the loop parameter was dropped from the get_session api. I have pushed the changes to our current aiobotocore tests in this PR. Your tests should be passing once this PR is merged.

@codecov-io
Copy link

codecov-io commented Mar 4, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #206      +/-   ##
==========================================
+ Coverage   84.21%   84.23%   +0.01%     
==========================================
  Files          78       78              
  Lines        3067     3070       +3     
==========================================
+ Hits         2583     2586       +3     
  Misses        484      484
Impacted Files Coverage Δ
aws_xray_sdk/ext/django/templates.py 91.3% <100%> (+1.3%) ⬆️

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 a839d7f...16a00d7. Read the comment docs.

Copy link
Contributor

@srprash srprash left a comment

Choose a reason for hiding this comment

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

Shouldn't we update the django version in tox.ini as well?

aws_xray_sdk/ext/django/templates.py Show resolved Hide resolved
@kmichel2 kmichel2 requested a review from srprash March 5, 2020 16:01
tox.ini Outdated Show resolved Hide resolved
Copy link
Contributor

@srprash srprash left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for contributing. :)

@srprash srprash merged commit ae3f213 into aws:master Mar 6, 2020
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.

4 participants