From 327028c4d91e1394df69be3dacf95d9617489c83 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 21 Jun 2018 11:17:54 +0200 Subject: [PATCH] Fix handling of optional guardian module Currently you cannot import something from rest_framework without configuring Django, since accessing `settings.INSTALLED_APPS` for the django-guardian throws an ImproperlyConfigured error: from rest_framework.exceptions import ErrorDetail ~/Vcs/django-rest-framework/rest_framework/exceptions.py in () 16 17 from rest_framework import status ---> 18 from rest_framework.compat import unicode_to_repr 19 from rest_framework.utils.serializer_helpers import ReturnDict, ReturnList 20 ~/Vcs/django-rest-framework/rest_framework/compat.py in () 144 guardian = None 145 try: --> 146 if 'guardian' in settings.INSTALLED_APPS: 147 import guardian # noqa 148 except ImportError: ~/Vcs/django-rest-framework/.venv/lib/python3.6/site-packages/django/conf/__init__.py in __getattr__(self, name) 54 """Return the value of a setting and cache it in self.__dict__.""" 55 if self._wrapped is empty: ---> 56 self._setup(name) 57 val = getattr(self._wrapped, name) 58 self.__dict__[name] = val ~/Vcs/django-rest-framework/.venv/lib/python3.6/site-packages/django/conf/__init__.py in _setup(self, name) 39 "You must either define the environment variable %s " 40 "or call settings.configure() before accessing settings." ---> 41 % (desc, ENVIRONMENT_VARIABLE)) 42 43 self._wrapped = Settings(settings_module) ImproperlyConfigured: Requested setting INSTALLED_APPS, but settings are not configured. You must either define the environment variable DJANGO_SETTINGS_MODULE or call settings.configure() before accessing settings. I've thought about just trying to import it always, but that is some overhead for something optional, and would also regress #1712 - except for when handling the `ImproperlyConfigured` from there. Since it only gets used by a single class this patch moves the code there. --- rest_framework/compat.py | 10 ---------- rest_framework/filters.py | 10 +++++++--- tests/test_permissions.py | 5 ++++- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/rest_framework/compat.py b/rest_framework/compat.py index 7783bced3..7db548f00 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -139,16 +139,6 @@ except ImportError: requests = None -# Django-guardian is optional. Import only if guardian is in INSTALLED_APPS -# Fixes (#1712). We keep the try/except for the test suite. -guardian = None -try: - if 'guardian' in settings.INSTALLED_APPS: - import guardian # noqa -except ImportError: - pass - - # PATCH method is not implemented by Django if 'patch' not in View.http_method_names: View.http_method_names = View.http_method_names + ['patch'] diff --git a/rest_framework/filters.py b/rest_framework/filters.py index cf8f6d1f7..3b52e5c19 100644 --- a/rest_framework/filters.py +++ b/rest_framework/filters.py @@ -16,7 +16,7 @@ from django.utils import six from django.utils.encoding import force_text from django.utils.translation import ugettext_lazy as _ -from rest_framework.compat import coreapi, coreschema, distinct, guardian +from rest_framework.compat import coreapi, coreschema, distinct from rest_framework.settings import api_settings @@ -282,7 +282,11 @@ class DjangoObjectPermissionsFilter(BaseFilterBackend): has read object level permissions. """ def __init__(self): - assert guardian, 'Using DjangoObjectPermissionsFilter, but django-guardian is not installed' + try: + import guardian + except ImportError: + raise ImportError('Using DjangoObjectPermissionsFilter, but django-guardian is not installed') + self.guardian_version = guardian.VERSION perm_format = '%(app_label)s.view_%(model_name)s' @@ -300,7 +304,7 @@ class DjangoObjectPermissionsFilter(BaseFilterBackend): 'model_name': model_cls._meta.model_name } permission = self.perm_format % kwargs - if tuple(guardian.VERSION) >= (1, 3): + if tuple(self.guardian_version) >= (1, 3): # Maintain behavior compatibility with versions prior to 1.3 extra = {'accept_global_perms': False} else: diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 80b666180..c39b4d12a 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -12,7 +12,10 @@ from rest_framework import ( HTTP_HEADER_ENCODING, authentication, generics, permissions, serializers, status, views ) -from rest_framework.compat import guardian +try: + import guardian +except ImportError: + guardian = None from rest_framework.filters import DjangoObjectPermissionsFilter from rest_framework.routers import DefaultRouter from rest_framework.test import APIRequestFactory