Django-rest-framework: Settings should be updated when overridden in Django tests

[jamescooke]

DRF has its own settings object at rest_framework.settings.api_settings. It appears that these are generated once and not updated when override_settings is used in Django testing.

Failing test

With this in a settings file:

REST_FRAMEWORK = {
    'PAGINATE_BY': 10,
}

This will fail:

from django.test.utils import override_settings
from rest_framework.settings import api_settings
from rest_framework.test import APITestCase


class TestSettings(TestCase):

    def test_settings_normal(self):
        self.assertEqual(settings.REST_FRAMEWORK['PAGINATE_BY'], 10)
        self.assertEqual(api_settings.PAGINATE_BY, 10)

    def test_settings_overridden(self):
        with override_settings(REST_FRAMEWORK={'PAGINATE_BY': 999}):
            self.assertEqual(settings.REST_FRAMEWORK['PAGINATE_BY'], 999)
            self.assertEqual(api_settings.PAGINATE_BY, 999)

test_settings_overridden FAILS because PAGINATE_BY in api_settings is still 10 during override, when it should have been overridden to 999.

This is unexpected behaviour - Django developers familiar with the Django settings and the settings object will expect to be able to override at testing time as per the Django docs.

Simple solutions?

There are probably some elegant solutions to wire in the build of api_settings to listen for updates made by the settings override functions in Django tests... however, what about using what's in DRF already?

There is a temporary_setting function in tests.utils: https://github.com/tomchristie/django-rest-framework/blob/master/tests/utils.py#L9 , which looks like it can do the job. If this could be included in the DRF package and documented then it could be a "good enough" solution maybe.

[tomchristie]

which looks like it can do the job

Looking at it I'd say we have that in the tests because it isn't really adequate to go into the core package.
I guess I'd rather see us do something that works with override_settings.

I guess we probably want to look at hooking into the setting_changed signal. https://docs.djangoproject.com/en/1.4/ref/signals/#django.test.signals.setting_changed

[tomchristie]

@jamescooke Bunch of thoughts on #2473. Appreciate any feedback you might have. Unclear how to proceed from here.

[adamchainz]

This got me too, commenting on #2473.

[tomchristie]

Okay, merged #2473, which resolves the api_settings object being updated. Now we need to refactor any places where we have class attributes accessing API settings. In the meantime a note in the testing docs on the (sometime) need to reload_module on test setup, test teardown may be worthwhile.

[tomchristie]

$ ack "^    [a-z_]* = api_settings[.]" ./rest_framework/
rest_framework/fields.py
734:     coerce_to_string = api_settings.COERCE_DECIMAL_TO_STRING
816:     format = api_settings.DATETIME_FORMAT
817:     input_formats = api_settings.DATETIME_INPUT_FORMATS
881:     format = api_settings.DATE_FORMAT
882:     input_formats = api_settings.DATE_INPUT_FORMATS
938:     format = api_settings.TIME_FORMAT
939:     input_formats = api_settings.TIME_INPUT_FORMATS
1073:    use_url = api_settings.UPLOADED_FILES_USE_URL

rest_framework/filters.py
75:     search_param = api_settings.SEARCH_PARAM
114:    ordering_param = api_settings.ORDERING_PARAM

rest_framework/generics.py
59:    paginate_by = api_settings.PAGINATE_BY
60:    paginate_by_param = api_settings.PAGINATE_BY_PARAM
61:    max_paginate_by = api_settings.MAX_PAGINATE_BY
62:    pagination_serializer_class = api_settings.DEFAULT_PAGINATION_SERIALIZER_CLASS
66:    filter_backends = api_settings.DEFAULT_FILTER_BACKENDS

rest_framework/renderers.py
61:    compact = api_settings.COMPACT_JSON

rest_framework/test.py
24:    renderer_classes_list = api_settings.TEST_REQUEST_RENDERER_CLASSES
25:    default_format = api_settings.TEST_REQUEST_DEFAULT_FORMAT

rest_framework/urlpatterns.py
52:    suffix_kwarg = api_settings.FORMAT_SUFFIX_KWARG

rest_framework/utils/breadcrumbs.py
14:    view_name_func = api_settings.VIEW_NAME_FUNCTION

rest_framework/views.py
89:    renderer_classes = api_settings.DEFAULT_RENDERER_CLASSES
90:    parser_classes = api_settings.DEFAULT_PARSER_CLASSES
91:    authentication_classes = api_settings.DEFAULT_AUTHENTICATION_CLASSES
92:    throttle_classes = api_settings.DEFAULT_THROTTLE_CLASSES
93:    permission_classes = api_settings.DEFAULT_PERMISSION_CLASSES
94:    content_negotiation_class = api_settings.DEFAULT_CONTENT_NEGOTIATION_CLASS
95:    metadata_class = api_settings.DEFAULT_METADATA_CLASS

[tomchristie]

See also #4324.

[daggaz]

Unless I'm missing something, as far as I can see the changes made in response to this issue so far do not address the original issue of override_settings not being respected.

The current reload_api_settings() signal handler sets the module level name api_settings in the settings module to a new APISettings instance, which is fine.

However, wherever api_settings is actually used (even in the code from #3363) the value is assigned to a local name in the importing module at import time. For example in fields.py:

from rest_framework.settings import api_settings

class DateField(Field):
    def to_representation(self, value):
        output_format = getattr(self, 'format', api_settings.DATE_FORMAT)

The name api_settings in fields.py will not be changed by reload_api_settings() and will always point at the APISettings instance imported at the time fields.py was imported.

[daggaz]

My suggestion to resolve this, while maintaining the current settings attribute caching is to add a reload() method to APISettings:

class APISettings(object):
    def __init__(self, ...):
        ...
        self._cached_attrs = set()

    def __getattr__(self, attr):
        ...
        self._cached_attrs.add(attr)
        return val

    def reload(self):
        for attr in self._cached_attrs:
            delattr(self, attr)
        self._cached_attrs.clear()
        if hasattr(self, '_user_settings'):
            delattr(self, '_user_settings')

Then change reload_api_settings() to:

def reload_api_settings(*args, **kwargs):
    if kwargs['setting'] == 'REST_FRAMEWORK':
        dynamic_settings.reload()

[carltongibson]

I just ran into this. It's behaving exactly as @daggaz describes.

[maccinza]

I have just ran into this issue, I'm trying to override the PAGE_SIZE settings:

@override_settings(REST_FRAMEWORK={"PAGE_SIZE": 20})
def test_my_test(self): 
    pass

But the api still responds with the pagination initially configured in the django settings module.

Still no clear way to properly override api settings within tests?
I had to directly mock the paginator class for overriding the its page_size attribute.

[Virako]

from django.conf import settings
from rest_framework.settings import api_settings


@override_settings(REST_FRAMEWORK={"PAGE_SIZE": 20})
def test_my_test(self): 
    page_size = settings.REST_FRAMEWORK.get('PAGE_SIZE')  # Not work
    page_size = api_settings.PAGE_SIZE  # Work