django: Fix instrumentation and tests for all Django major versions
#780
Conversation
tox.ini
Outdated
| py3{6,7,8,9}-test-instrumentation-django | ||
| pypy3-test-instrumentation-django | ||
| py3{6,7,8,9}-test-instrumentation-django{1,2,3,4} | ||
| pypy3-test-instrumentation-django{1,2,3,4} |
There was a problem hiding this comment.
This is fine for now but we should think if we can reduce the matrix size. May be test every single version on the oldest supported python version and then test only latest (or a random) version on every other python version. Concern is increased build times but this one change shouldn't make that much of a difference so no urgency yet.
There was a problem hiding this comment.
Now that you mention it, we could at least reduce the amount of combination by testing only the officially supported Python version for each Django major release. That would end up in 4 removed test environments, considering that:
- Django 1.11 officially supports up to Python 3.7: https://docs.djangoproject.com/en/3.2/releases/1.11/
- Django 4.0 officially supports Python 3.8 and newer: https://code.djangoproject.com/ticket/32355
py3{6,7}-test-instrumentation-django1
py3{6,7,8,9}-test-instrumentation-django{2,3}
py3{8,9}-test-instrumentation-django4
pypy3-test-instrumentation-django{1,2,3,4}
There was a problem hiding this comment.
pypy3 is currently based on Python 3.7, so it's not able to install Django 4.x.
There was a problem hiding this comment.
@adamantike
Could you add a comment inline to explain this reasoning?
There was a problem hiding this comment.
Good call! I've added a note, let me know if it provides all the needed context.
* Fix support for Django `1.10`/`1.11`, by allowing `settings.MIDDLEWARE`. * Add Tox environments for every Django major version, including 4.0 beta. * Fix tests for new Tox environments to pass. Relevant context: * In Django 1.x, usage of `settings.MIDDLEWARE_CLASSES` is detected by running `settings.MIDDLEWARE is None`: https://github.com/django/django/blob/58f02c498b659a906e9c30d946bd89bedc4717e5/django/core/handlers/base.py#L48 * HTTP route in `ResolverMatch` object is only available since Django 2.2: django/django#10657 * Django class `HttpResponseBase` works as a mapping object, so avoiding accessing its internal headers object simplifies support accross Django versions.
adamantike
force-pushed
the
fix-django-legacy-support
branch
from
f900037 to
a0ed547
Compare
Description
1.10/1.11, by allowingsettings.MIDDLEWARE_CLASSES.Relevant context:
settings.MIDDLEWARE_CLASSESis detected by runningsettings.MIDDLEWARE is None: https://github.com/django/django/blob/58f02c498b659a906e9c30d946bd89bedc4717e5/django/core/handlers/base.py#L48ResolverMatchobject is only available since Django 2.2: Fixed #28766 -- Added ResolverMatch.route django/django#10657HttpResponseBaseworks as a mapping object, so avoiding accessing its internal headers object simplifies support across Django versions.Type of change
How Has This Been Tested?
tox -e test-instrumentation-django{1,2,3,4}Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.