Skip to content

Conversation

lovelydinosaur
Copy link
Contributor

@lovelydinosaur lovelydinosaur commented Jun 1, 2016

Working towards Django 1.10 support.

@@ -635,6 +635,7 @@ def get_context(self, data, accepted_media_type, renderer_context):
'view': view,
'request': request,
'response': response,
'user': request.user,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed by Django 1.10 ?

Copy link
Contributor Author

@lovelydinosaur lovelydinosaur Jun 1, 2016

Choose a reason for hiding this comment

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

For one reason or another, yes.

I've not dug into why yet, but presumably we were previously having it automatically included by the RequestContext. I assume that we could also change our settings in order to resolve this (eg perhaps context processor configuration has moved to being part of the TEMPLATES dictionary?)

In any case it's probably best that we pass it explicitly, rather than relying on the correct context processors being set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Verified. Previously we were relying on the default value of TEMPLATE_CONTEXT_PROCESSORS, which includes django.contrib.auth.context_processors.auth.

We could add this in to the TEMPLATES.OPTIONS.context_processors in conftest.py, which would resolve our test cases, but it's better if we pass it explicitly, and not rely on the user settings.

@codecov-io
Copy link

codecov-io commented Jun 1, 2016

Current coverage is 91.47%

Merging #4158 into master will decrease coverage by <.01%

@@             master      #4158   diff @@
==========================================
  Files            51         51          
  Lines          5483       5487     +4   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           5016       5019     +3   
- Misses          467        468     +1   
  Partials          0          0          

Powered by Codecov. Last updated by fe2aede...c65c5e4

@lovelydinosaur
Copy link
Contributor Author

There's an outstanding issue, downstream in django-filters, which uses get_field_by_name which does not exist across all versions. Once that's resolved we can remove the "expected failure" marker.

ERROR collecting tests/test_filters.py
=====================
tests/test_filters.py:40: in <module>
    class SeveralFieldsFilter(django_filters.FilterSet):
env/lib/python2.7/site-packages/django_filters/filterset.py:197: in __new__
    new_class.filter_for_reverse_field)
env/lib/python2.7/site-packages/django_filters/filterset.py:111: in filters_for_model
    field = get_model_field(model, f)
env/lib/python2.7/site-packages/django_filters/filterset.py:93: in get_model_field
    rel, model, direct, m2m = opts.get_field_by_name(parts[-1])
E   AttributeError: 'Options' object has no attribute 'get_field_by_name'

In the meantime there's no reason not to merge this now, as it resolves almost all our issues, and the remaining failure is in the "expected failures" matrix.

@lovelydinosaur lovelydinosaur merged commit 994e1ba into master Jun 1, 2016
@lovelydinosaur lovelydinosaur deleted the django-1.10 branch June 1, 2016 14:31
@carltongibson
Copy link
Collaborator

I'll pick that up.

@lovelydinosaur
Copy link
Contributor Author

👍 Let me know if you want some of my time, too.

@carltongibson
Copy link
Collaborator

Only if it's hard. :-) — It should be OK.

rlucioni pushed a commit to openedx/course-discovery that referenced this pull request Jul 12, 2017
DRF 3.4.7 is the same version run by edx/credentials. Release notes are at http://www.django-rest-framework.org/topics/release-notes/#34x-series. Highlights include: support for Django 1.10 (encode/django-rest-framework#4158 - sigh), support for schema generation (encode/django-rest-framework#4179 - required for newer versions of django-rest-swagger), and a change that prevents paginated views from re-running queries when count queries return 0 (encode/django-rest-framework#4201 - this explains the expected query count changes made in tests).

LEARNER-1590
rlucioni pushed a commit to openedx/course-discovery that referenced this pull request Jul 12, 2017
DRF 3.4.7 is the same version run by edx/credentials. Release notes are at http://www.django-rest-framework.org/topics/release-notes/#34x-series.

Highlights include:
    - [Support for Django 1.10](encode/django-rest-framework#4158) - sigh
    - [Support for schema generation](encode/django-rest-framework#4179) - required for newer versions of django-rest-swagger
    - A [change](encode/django-rest-framework#4201) that prevents paginated views from re-running queries when count queries return 0 - this explains the expected query count changes made in tests

LEARNER-1590
rlucioni pushed a commit to openedx/course-discovery that referenced this pull request Jul 12, 2017
DRF 3.4.7 is the same version run by edx/credentials. Release notes are at http://www.django-rest-framework.org/topics/release-notes/#34x-series. Highlights include:

1. [Support for Django 1.10](encode/django-rest-framework#4158) - sigh
2. [Support for schema generation](encode/django-rest-framework#4179) - required for newer versions of django-rest-swagger
3. A change that [prevents paginated views from re-running queries](encode/django-rest-framework#4201) when count queries return 0 - this explains the expected query count changes made in tests

LEARNER-1590
rlucioni pushed a commit to openedx/course-discovery that referenced this pull request Jul 12, 2017
DRF 3.4.7 is the same version run by edx/credentials. Release notes are at http://www.django-rest-framework.org/topics/release-notes/#34x-series. Highlights include:

1. [Support for Django 1.10](encode/django-rest-framework#4158) - sigh
2. [Support for schema generation](encode/django-rest-framework#4179) - required for newer versions of django-rest-swagger
3. A change that [prevents paginated views from re-running queries](encode/django-rest-framework#4201) when count queries return 0 - this explains the expected query count changes made in tests

LEARNER-1590
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.

4 participants