-
-
Notifications
You must be signed in to change notification settings - Fork 144
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
Move to PEP-451 style loader #386
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
import importlib.util | ||
from importlib.machinery import PathFinder | ||
import logging | ||
import os | ||
|
@@ -47,12 +46,12 @@ | |
return parser | ||
|
||
base.BaseCommand.create_parser = create_parser | ||
importer = ConfigurationImporter(check_options=check_options) | ||
importer = ConfigurationFinder(check_options=check_options) | ||
sys.meta_path.insert(0, importer) | ||
installed = True | ||
|
||
|
||
class ConfigurationImporter: | ||
class ConfigurationFinder(PathFinder): | ||
modvar = SETTINGS_ENVIRONMENT_VARIABLE | ||
namevar = CONFIGURATION_ENVIRONMENT_VARIABLE | ||
error_msg = ("Configuration cannot be imported, " | ||
|
@@ -71,7 +70,7 @@ | |
self.announce() | ||
|
||
def __repr__(self): | ||
return "<ConfigurationImporter for '{}.{}'>".format(self.module, | ||
return "<ConfigurationFinder for '{}.{}'>".format(self.module, | ||
self.name) | ||
|
||
@property | ||
|
@@ -129,56 +128,53 @@ | |
|
||
def find_spec(self, fullname, path=None, target=None): | ||
if fullname is not None and fullname == self.module: | ||
spec = PathFinder.find_spec(fullname, path) | ||
spec = super().find_spec(fullname, path, target) | ||
if spec is not None: | ||
return importlib.machinery.ModuleSpec(spec.name, | ||
ConfigurationLoader(self.name, spec), | ||
origin=spec.origin) | ||
return None | ||
|
||
|
||
class ConfigurationLoader: | ||
|
||
def __init__(self, name, spec): | ||
self.name = name | ||
self.spec = spec | ||
|
||
def load_module(self, fullname): | ||
if fullname in sys.modules: | ||
mod = sys.modules[fullname] # pragma: no cover | ||
wrap_loader(spec.loader, self.name) | ||
return spec | ||
else: | ||
mod = importlib.util.module_from_spec(self.spec) | ||
sys.modules[fullname] = mod | ||
self.spec.loader.exec_module(mod) | ||
|
||
cls_path = f'{mod.__name__}.{self.name}' | ||
|
||
try: | ||
cls = getattr(mod, self.name) | ||
except AttributeError as err: # pragma: no cover | ||
reraise(err, "Couldn't find configuration '{}' " | ||
"in module '{}'".format(self.name, | ||
mod.__package__)) | ||
try: | ||
cls.pre_setup() | ||
cls.setup() | ||
obj = cls() | ||
attributes = uppercase_attributes(obj).items() | ||
for name, value in attributes: | ||
if callable(value) and not getattr(value, 'pristine', False): | ||
value = value() | ||
# in case a method returns a Value instance we have | ||
# to do the same as the Configuration.setup method | ||
if isinstance(value, Value): | ||
setup_value(mod, name, value) | ||
continue | ||
setattr(mod, name, value) | ||
|
||
setattr(mod, 'CONFIGURATION', '{}.{}'.format(fullname, | ||
self.name)) | ||
cls.post_setup() | ||
|
||
except Exception as err: | ||
reraise(err, f"Couldn't setup configuration '{cls_path}'") | ||
|
||
return mod | ||
return None | ||
|
||
|
||
def wrap_loader(loader, class_name): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bonus of using this wrapping approach is that it should still work when the settings file is a frozen module, or otherwise provided by some special kind of module. I don’t believe that was supported before, because the old |
||
class ConfigurationLoader(loader.__class__): | ||
def exec_module(self, module): | ||
super().exec_module(module) | ||
|
||
mod = module | ||
|
||
cls_path = f'{mod.__name__}.{class_name}' | ||
|
||
try: | ||
cls = getattr(mod, class_name) | ||
except AttributeError as err: # pragma: no cover | ||
reraise( | ||
err, | ||
( | ||
f"Couldn't find configuration '{class_name}' in " | ||
f"module '{mod.__package__}'" | ||
), | ||
) | ||
try: | ||
cls.pre_setup() | ||
cls.setup() | ||
obj = cls() | ||
attributes = uppercase_attributes(obj).items() | ||
for name, value in attributes: | ||
if callable(value) and not getattr(value, 'pristine', False): | ||
value = value() | ||
# in case a method returns a Value instance we have | ||
# to do the same as the Configuration.setup method | ||
if isinstance(value, Value): | ||
setup_value(mod, name, value) | ||
continue | ||
setattr(mod, name, value) | ||
|
||
setattr(mod, 'CONFIGURATION', '{0}.{1}'.format(module.__name__, | ||
class_name)) | ||
cls.post_setup() | ||
|
||
except Exception as err: | ||
reraise(err, f"Couldn't setup configuration '{cls_path}'") | ||
|
||
loader.__class__ = ConfigurationLoader |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
from configurations import Configuration | ||
|
||
|
||
class ErrorConfiguration(Configuration): | ||
|
||
@classmethod | ||
def pre_setup(cls): | ||
raise ValueError("Error in pre_setup") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import os | ||
from django.test import TestCase | ||
from unittest.mock import patch | ||
|
||
|
||
class ErrorTests(TestCase): | ||
|
||
@patch.dict(os.environ, clear=True, | ||
DJANGO_CONFIGURATION='ErrorConfiguration', | ||
DJANGO_SETTINGS_MODULE='tests.settings.error') | ||
def test_env_loaded(self): | ||
with self.assertRaises(ValueError) as cm: | ||
from tests.settings import error # noqa: F401 | ||
|
||
self.assertIsInstance(cm.exception, ValueError) | ||
self.assertEqual( | ||
cm.exception.args, | ||
( | ||
"Couldn't setup configuration " | ||
"'tests.settings.error.ErrorConfiguration': Error in pre_setup ", | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also changing this class into a PEP-451 compliant finder, and renaming it as such.