From ea2a48838bd52d6db9b158b0182449f47b6fbbd7 Mon Sep 17 00:00:00 2001 From: plup <plup@plup.io> Date: Fri, 2 Oct 2020 19:10:26 +0200 Subject: [PATCH 1/7] feat: cached settings resolution at init --- djangoldp/conf/settings.py | 118 ++++++++++++++++++++----------------- 1 file changed, 64 insertions(+), 54 deletions(-) diff --git a/djangoldp/conf/settings.py b/djangoldp/conf/settings.py index 4a486945..1ee751e7 100644 --- a/djangoldp/conf/settings.py +++ b/djangoldp/conf/settings.py @@ -21,8 +21,9 @@ def configure(filename='settings.yml'): """Helper function to configure django from LDPSettings.""" # ref: https://docs.djangoproject.com/fr/2.2/topics/settings/#custom-default-settings - settings = LDPSettings(path=filename) - django_settings.configure(settings) # gives a LazySettings + ldpsettings = LDPSettings(path=filename) + + django_settings.configure(ldpsettings) class LDPSettings(object): @@ -36,6 +37,7 @@ class LDPSettings(object): self.path = path self._config = None + self._settings = self.build_settings() @property def config(self): @@ -65,12 +67,12 @@ class LDPSettings(object): # import from an installed package mod = import_module(f'{pkg}.djangoldp_settings') logger.debug(f'Settings found for {pkg} in a installed package') - except (ModuleNotFoundError): + except ModuleNotFoundError: try: # import from a local packages in a subfolder (same name the template is built this way) mod = import_module(f'{pkg}.{pkg}.djangoldp_settings') logger.debug(f'Settings found for {pkg} in a local package') - except (ModuleNotFoundError): + except ModuleNotFoundError: logger.debug(f'No settings found for {pkg}') break @@ -78,12 +80,64 @@ class LDPSettings(object): try: attr.extend(getattr(mod, attributes)) logger.debug(f'{attributes} found in local package {pkg}') - except (NameError): + except NameError: logger.info(f'No {attributes} found for package {pkg}') pass return attr + def build_settings(self): + """ + Look for the parameter in config. Each step override the value of the previous key found. + + Resolution order of the configuration: + 1. Core default settings + 2. Packages settings + 3. Code from a local settings.py file + 4. YAML config file + """ + + # start from default core settings + settings = global_settings.__dict__ + logger.debug(f'building settings from core defaults') + + # look settings from packages in the order they are given (local override installed) + for pkg in self.DJANGOLDP_PACKAGES: + + try: + # override with values from installed package + mod = import_module(f'{pkg}.djangoldp_settings') + settings.update({k: v for k, v in mod.__dict__.items() if not k.startswith('_')}) + logger.debug(f'updating settings from installed package {pkg}') + except ModuleNotFoundError: + pass + + try: + # override with values from local package + mod = import_module(f'{pkg}.{pkg}.djangoldp_settings') + settings.update({k: v for k, v in mod.__dict__.items() if not k.startswith('_')}) + logger.debug(f'updating settings from local package {pkg}') + except ModuleNotFoundError: + pass + + # look in settings.py file in directory + try: + mod = import_module('settings') + settings.update({k: v for k, v in mod.__dict__.items() if not k.startswith('_')}) + logger.debug(f'updating settings from local settings.py file') + except ModuleNotFoundError: + pass + + # look in YAML config file 'server' section + try: + conf = self.config.get('server', {}) + settings.update({k: v for k, v in conf.items() if not k.startswith('_')}) + logger.debug(f'updating settings with project config') + except KeyError: + pass + + return settings + @property def DJANGOLDP_PACKAGES(self): @@ -124,54 +178,10 @@ class LDPSettings(object): return middlewares def __getattr__(self, param): - - """ - Look for the parameter in config and return the first value found. - - Resolution order of the configuration: - 1. YAML config file - 2. Packages settings - 3. Core default settings - """ - - if not param.startswith('_') and param.isupper(): - - # look in config file - try: - value = self.config['server'][param] - logger.debug(f'{param} found in project config') - return value - except KeyError: - pass - - # look in all packages config - for pkg in self.DJANGOLDP_PACKAGES: - - try: - # import from local package - mod = import_module(f'{pkg}.{pkg}.djangoldp_settings') - value = getattr(mod, param) - logger.debug(f'{param} found in local package {pkg}') - return value - except (ModuleNotFoundError, NameError, AttributeError): - pass - - try: - # import from installed package - mod = import_module(f'{pkg}.djangoldp_settings') - value = getattr(mod, param) - logger.debug(f'{param} found in installed package {pkg}') - return value - except (ModuleNotFoundError, NameError, AttributeError): - pass - - # look in default settings - try: - value = getattr(global_settings, param) - logger.debug(f'{param} found in core default config') - return value - except AttributeError: - pass - + """Return the requested parameter from cached settings.""" + try: + return self._settings[param] + except KeyError: # raise the django exception for inexistent parameter raise AttributeError(f'no "{param}" parameter found in settings') + -- GitLab From 86f2777a4f4bd29be2941064890c80895e847aba Mon Sep 17 00:00:00 2001 From: plup <plup@plup.io> Date: Fri, 2 Oct 2020 19:30:11 +0200 Subject: [PATCH 2/7] feat: renamed settings.py to limit confusion --- djangoldp/conf/{settings.py => ldpsettings.py} | 2 ++ djangoldp/conf/server_template/manage.py-tpl | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) rename djangoldp/conf/{settings.py => ldpsettings.py} (99%) diff --git a/djangoldp/conf/settings.py b/djangoldp/conf/ldpsettings.py similarity index 99% rename from djangoldp/conf/settings.py rename to djangoldp/conf/ldpsettings.py index 1ee751e7..6b16255e 100644 --- a/djangoldp/conf/settings.py +++ b/djangoldp/conf/ldpsettings.py @@ -172,6 +172,8 @@ class LDPSettings(object): # get default middlewares middlewares = getattr(global_settings, 'MIDDLEWARE') + print(middlewares) + # explore packages looking for middleware to reference middlewares.extend(self.fetch('MIDDLEWARE')) diff --git a/djangoldp/conf/server_template/manage.py-tpl b/djangoldp/conf/server_template/manage.py-tpl index cbeca750..8cc40650 100755 --- a/djangoldp/conf/server_template/manage.py-tpl +++ b/djangoldp/conf/server_template/manage.py-tpl @@ -1,7 +1,7 @@ #!/usr/bin/env python import os import sys -from djangoldp.conf import settings as ldpsettings +from djangoldp.conf import ldpsettings if __name__ == "__main__": -- GitLab From 79f1480e85885d38d1c9246d4ec5c5bfc9817103 Mon Sep 17 00:00:00 2001 From: plup <plup@plup.io> Date: Fri, 2 Oct 2020 20:26:48 +0200 Subject: [PATCH 3/7] fix: refactored access to MIDDLEWARE to avoid concurrent writing --- djangoldp/conf/ldpsettings.py | 95 ++++++------------- .../app_name/djangoldp_settings.py-tpl | 3 + .../conf/server_template/server/wsgi.py-tpl | 2 +- djangoldp/tests/runner.py | 2 +- 4 files changed, 34 insertions(+), 68 deletions(-) diff --git a/djangoldp/conf/ldpsettings.py b/djangoldp/conf/ldpsettings.py index 6b16255e..6e5a24a4 100644 --- a/djangoldp/conf/ldpsettings.py +++ b/djangoldp/conf/ldpsettings.py @@ -45,8 +45,11 @@ class LDPSettings(object): """Load configuration from file.""" if not self._config: - with open(self.path, 'r') as f: - self._config = yaml.safe_load(f) + try: + with open(self.path, 'r') as f: + self._config = yaml.safe_load(f) + except FileNotFoundError: + logger.info('Starting project without configuration file') return self._config @@ -55,40 +58,10 @@ class LDPSettings(object): """Set a dict has current configuration.""" self._config = value - def fetch(self, attributes): - """ - Explore packages looking for a list of attributes within the server configuration. - It returns all elements found and doesn't manage duplications or collisions. + def build_settings(self, extend=['INSTALLED_APPS', 'MIDDLEWARE']): """ - - attr = [] - for pkg in self.DJANGOLDP_PACKAGES: - try: - # import from an installed package - mod = import_module(f'{pkg}.djangoldp_settings') - logger.debug(f'Settings found for {pkg} in a installed package') - except ModuleNotFoundError: - try: - # import from a local packages in a subfolder (same name the template is built this way) - mod = import_module(f'{pkg}.{pkg}.djangoldp_settings') - logger.debug(f'Settings found for {pkg} in a local package') - except ModuleNotFoundError: - logger.debug(f'No settings found for {pkg}') - break - - # looking for the attribute list in the module - try: - attr.extend(getattr(mod, attributes)) - logger.debug(f'{attributes} found in local package {pkg}') - except NameError: - logger.info(f'No {attributes} found for package {pkg}') - pass - - return attr - - def build_settings(self): - """ - Look for the parameter in config. Each step override the value of the previous key found. + Look for the parameters in multiple places. + Each step overrides the value of the previous key found. Except for "extend" list. Those value must be lists and all values found are added to these lists without managing duplications. Resolution order of the configuration: 1. Core default settings @@ -97,9 +70,19 @@ class LDPSettings(object): 4. YAML config file """ + # helper loop + def update_with(config): + for k, v in config.items(): + + if k in extend: + settings[k].extend(v) + + elif not k.startswith('_'): + settings.update({k: v}) + # start from default core settings settings = global_settings.__dict__ - logger.debug(f'building settings from core defaults') + logger.debug(f'Building settings from core defaults') # look settings from packages in the order they are given (local override installed) for pkg in self.DJANGOLDP_PACKAGES: @@ -107,32 +90,32 @@ class LDPSettings(object): try: # override with values from installed package mod = import_module(f'{pkg}.djangoldp_settings') - settings.update({k: v for k, v in mod.__dict__.items() if not k.startswith('_')}) - logger.debug(f'updating settings from installed package {pkg}') + update_with(mod.__dict__) + logger.debug(f'Updating settings from installed package {pkg}') except ModuleNotFoundError: pass try: # override with values from local package mod = import_module(f'{pkg}.{pkg}.djangoldp_settings') - settings.update({k: v for k, v in mod.__dict__.items() if not k.startswith('_')}) - logger.debug(f'updating settings from local package {pkg}') + update_with(mod.__dict__) + logger.debug(f'Updating settings from local package {pkg}') except ModuleNotFoundError: pass # look in settings.py file in directory try: mod = import_module('settings') - settings.update({k: v for k, v in mod.__dict__.items() if not k.startswith('_')}) - logger.debug(f'updating settings from local settings.py file') + update_with(mod.__dict__) + logger.debug(f'Updating settings from local settings.py file') except ModuleNotFoundError: pass # look in YAML config file 'server' section try: conf = self.config.get('server', {}) - settings.update({k: v for k, v in conf.items() if not k.startswith('_')}) - logger.debug(f'updating settings with project config') + update_with(conf) + logger.debug(f'Updating settings with project config') except KeyError: pass @@ -149,36 +132,16 @@ class LDPSettings(object): @property def INSTALLED_APPS(self): - """Return the default installed apps and the LDP packages.""" + """Return the installed apps and the LDP packages.""" # get default apps - apps = getattr(global_settings, 'INSTALLED_APPS') + apps = self._settings['INSTALLED_APPS'] # add ldp packages themselves (they are django apps) apps.extend(self.DJANGOLDP_PACKAGES) - # add apps referenced in packages - apps.extend(self.fetch('INSTALLED_APPS')) - return apps - @property - def MIDDLEWARE(self): - - """ - Return the default middlewares and the middlewares found in each LDP packages. - """ - - # get default middlewares - middlewares = getattr(global_settings, 'MIDDLEWARE') - - print(middlewares) - - # explore packages looking for middleware to reference - middlewares.extend(self.fetch('MIDDLEWARE')) - - return middlewares - def __getattr__(self, param): """Return the requested parameter from cached settings.""" try: diff --git a/djangoldp/conf/package_template/app_name/djangoldp_settings.py-tpl b/djangoldp/conf/package_template/app_name/djangoldp_settings.py-tpl index a0044498..879d0630 100644 --- a/djangoldp/conf/package_template/app_name/djangoldp_settings.py-tpl +++ b/djangoldp/conf/package_template/app_name/djangoldp_settings.py-tpl @@ -6,3 +6,6 @@ MYPACKAGE_VAR = 'MY_DEFAULT_VAR' # register an extra middleware MIDDLEWARE = [] + +# register an extra installed app +INSTALLED_APPS = [] diff --git a/djangoldp/conf/server_template/server/wsgi.py-tpl b/djangoldp/conf/server_template/server/wsgi.py-tpl index f2f21909..a84cf2a1 100644 --- a/djangoldp/conf/server_template/server/wsgi.py-tpl +++ b/djangoldp/conf/server_template/server/wsgi.py-tpl @@ -11,7 +11,7 @@ import os from django.core.wsgi import get_wsgi_application from django.conf import settings as django_settings -from djangoldp.conf import settings as ldpsettings +from djangoldp.conf import ldpsettings if not django_settings.configured: ldpsettings.configure() diff --git a/djangoldp/tests/runner.py b/djangoldp/tests/runner.py index 2e0e785c..de231164 100644 --- a/djangoldp/tests/runner.py +++ b/djangoldp/tests/runner.py @@ -3,7 +3,7 @@ import yaml import django from django.conf import settings as django_settings -from djangoldp.conf.settings import LDPSettings +from djangoldp.conf.ldpsettings import LDPSettings from djangoldp.tests.settings_default import yaml_config # override config loading -- GitLab From 7f1efb9d7c6c17ba503db361e264b048fe60d1a6 Mon Sep 17 00:00:00 2001 From: plup <plup@plup.io> Date: Tue, 6 Oct 2020 16:54:49 +0200 Subject: [PATCH 4/7] bugfix: extracted YAML loading from settings class --- djangoldp/conf/ldpsettings.py | 36 +++++++++++------------------------ djangoldp/tests/runner.py | 6 +++--- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/djangoldp/conf/ldpsettings.py b/djangoldp/conf/ldpsettings.py index 6e5a24a4..74f1fa2f 100644 --- a/djangoldp/conf/ldpsettings.py +++ b/djangoldp/conf/ldpsettings.py @@ -20,8 +20,14 @@ logger = logging.getLogger(__name__) def configure(filename='settings.yml'): """Helper function to configure django from LDPSettings.""" + try: + with open(self.path, 'r') as f: + config = yaml.safe_load(f) + except FileNotFoundError: + logger.info('Starting project without configuration file') + # ref: https://docs.djangoproject.com/fr/2.2/topics/settings/#custom-default-settings - ldpsettings = LDPSettings(path=filename) + ldpsettings = LDPSettings(config) django_settings.configure(ldpsettings) @@ -30,34 +36,14 @@ class LDPSettings(object): """Class managing the DjangoLDP configuration.""" - def __init__(self, path): + def __init__(self, config): if django_settings.configured: raise ImproperlyConfigured('Settings have been configured already') - self.path = path - self._config = None + self._config = config self._settings = self.build_settings() - @property - def config(self): - - """Load configuration from file.""" - - if not self._config: - try: - with open(self.path, 'r') as f: - self._config = yaml.safe_load(f) - except FileNotFoundError: - logger.info('Starting project without configuration file') - - return self._config - - @config.setter - def config(self, value): - """Set a dict has current configuration.""" - self._config = value - def build_settings(self, extend=['INSTALLED_APPS', 'MIDDLEWARE']): """ Look for the parameters in multiple places. @@ -113,7 +99,7 @@ class LDPSettings(object): # look in YAML config file 'server' section try: - conf = self.config.get('server', {}) + conf = self._config.get('server', {}) update_with(conf) logger.debug(f'Updating settings with project config') except KeyError: @@ -126,7 +112,7 @@ class LDPSettings(object): """Returns the list of LDP packages configured.""" - pkg = self.config.get('ldppackages', []) + pkg = self._config.get('ldppackages', []) return [] if pkg is None else pkg @property diff --git a/djangoldp/tests/runner.py b/djangoldp/tests/runner.py index de231164..db14a3a3 100644 --- a/djangoldp/tests/runner.py +++ b/djangoldp/tests/runner.py @@ -6,9 +6,9 @@ from django.conf import settings as django_settings from djangoldp.conf.ldpsettings import LDPSettings from djangoldp.tests.settings_default import yaml_config -# override config loading -ldpsettings = LDPSettings("") -ldpsettings.config = yaml.safe_load(yaml_config) +# load test config +config = yaml.safe_load(yaml_config) +ldpsettings = LDPSettings(config) django_settings.configure(ldpsettings) django.setup() -- GitLab From 67cc75c8faa9035656c8eff006b2f99a32c37bb8 Mon Sep 17 00:00:00 2001 From: plup <plup@plup.io> Date: Tue, 6 Oct 2020 16:58:18 +0200 Subject: [PATCH 5/7] test: fixed inversion in list comparison --- djangoldp/tests/tests_settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/djangoldp/tests/tests_settings.py b/djangoldp/tests/tests_settings.py index 7acf97f6..f561a880 100644 --- a/djangoldp/tests/tests_settings.py +++ b/djangoldp/tests/tests_settings.py @@ -38,8 +38,8 @@ class TestSettings(TestCase): 'django.contrib.staticfiles', 'djangoldp', 'guardian', - 'djangoldp.tests', - 'djangoldp.tests.dummy.apps.DummyConfig' + 'djangoldp.tests.dummy.apps.DummyConfig', + 'djangoldp.tests' ] def test_reference_middleware(self): -- GitLab From b7083fa6b83e57ec3f6894a56c8c2f101fe03b98 Mon Sep 17 00:00:00 2001 From: plup <plup@plup.io> Date: Tue, 6 Oct 2020 17:09:15 +0200 Subject: [PATCH 6/7] test: implemented missing tests for config overrides --- djangoldp/tests/djangoldp_settings.py | 3 ++- djangoldp/tests/settings_default.py | 1 + djangoldp/tests/tests_settings.py | 6 ++---- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/djangoldp/tests/djangoldp_settings.py b/djangoldp/tests/djangoldp_settings.py index ee6012e4..ca467b15 100644 --- a/djangoldp/tests/djangoldp_settings.py +++ b/djangoldp/tests/djangoldp_settings.py @@ -5,8 +5,9 @@ It contains configuration elements imported by djangoldp when the django server is setup. """ -# define an extra variable +# define an extra variables MYPACKAGEVAR = 'ok' +USE_I18N = False # register an extra middleware MIDDLEWARE = [ diff --git a/djangoldp/tests/settings_default.py b/djangoldp/tests/settings_default.py index 1a515021..fcbe93a7 100644 --- a/djangoldp/tests/settings_default.py +++ b/djangoldp/tests/settings_default.py @@ -47,6 +47,7 @@ server: DEFAULT_PAGINATION_CLASS: djangoldp.pagination.LDPPagination PAGE_SIZE: 5 USE_ETAGS: true + USE_TZ: false DEFAULT_CONTENT_TYPE: text/html FILE_CHARSET: utf-8 SEND_BACKLINKS: False diff --git a/djangoldp/tests/tests_settings.py b/djangoldp/tests/tests_settings.py index f561a880..96b7c9c3 100644 --- a/djangoldp/tests/tests_settings.py +++ b/djangoldp/tests/tests_settings.py @@ -16,12 +16,10 @@ class TestSettings(TestCase): assert settings.DJANGOLDP_PACKAGES == ['djangoldp.tests'] def test_overrided_core_by_package_config(self): - # FIXME - pass + assert settings.USE_I18N == False def test_overrided_package_by_user_config(self): - # FIXME - pass + assert settings.USE_TZ == False def test_overrided_core_by_user_config(self): """Asserts values overrided from user configuration.""" -- GitLab From ce589f49a079583ba52987aab21b2f1cee243ba7 Mon Sep 17 00:00:00 2001 From: plup <plup@plup.io> Date: Tue, 6 Oct 2020 19:25:31 +0200 Subject: [PATCH 7/7] bugfix: fixed opening with filename --- djangoldp/conf/ldpsettings.py | 4 +++- djangoldp/tests/tests_settings.py | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/djangoldp/conf/ldpsettings.py b/djangoldp/conf/ldpsettings.py index 74f1fa2f..868c0b1a 100644 --- a/djangoldp/conf/ldpsettings.py +++ b/djangoldp/conf/ldpsettings.py @@ -21,7 +21,7 @@ def configure(filename='settings.yml'): """Helper function to configure django from LDPSettings.""" try: - with open(self.path, 'r') as f: + with open(filename, 'r') as f: config = yaml.safe_load(f) except FileNotFoundError: logger.info('Starting project without configuration file') @@ -38,6 +38,8 @@ class LDPSettings(object): def __init__(self, config): + """Build a Django Setting object from a dict.""" + if django_settings.configured: raise ImproperlyConfigured('Settings have been configured already') diff --git a/djangoldp/tests/tests_settings.py b/djangoldp/tests/tests_settings.py index 96b7c9c3..aabe187b 100644 --- a/djangoldp/tests/tests_settings.py +++ b/djangoldp/tests/tests_settings.py @@ -52,3 +52,7 @@ class TestSettings(TestCase): 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'djangoldp.tests.dummy.middleware.DummyMiddleware' ] + + def test_extra_module(self): + #FIXME + pass -- GitLab