From 879c01ba006460849e222c17ce5a6b22f7f2fbb5 Mon Sep 17 00:00:00 2001 From: Alex Liebowitz Date: Thu, 1 Dec 2016 08:15:22 -0500 Subject: [PATCH 1/9] Add separate list of excluded fields to AdjustableSettings Adds "environ" field. Also renames __excluded to _excluded so it can be overridden by child classes. --- lbrynet/conf.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lbrynet/conf.py b/lbrynet/conf.py index 1f840e1b4..180921b30 100644 --- a/lbrynet/conf.py +++ b/lbrynet/conf.py @@ -45,11 +45,11 @@ else: class Settings(object): """A collection of configuration settings""" __fixed = [] - __excluded = ['get_dict', 'update'] + _excluded = ['get_dict', 'update'] def __iter__(self): for k in self.__dict__.iterkeys(): - if k.startswith('_') or k in self.__excluded: + if k.startswith('_') or k in self._excluded: continue yield k @@ -180,6 +180,8 @@ ENVIRONMENT = Env( class AdjustableSettings(Settings): + _excluded = ['get_dict', 'update', 'environ'] + """Settings that are allowed to be overriden by the user""" def __init__(self, environ=None): self.environ = environ or ENVIRONMENT From ef8d1cfdc2fe84c12a03450b53587cf78ed4efda Mon Sep 17 00:00:00 2001 From: Alex Liebowitz Date: Thu, 1 Dec 2016 08:21:37 -0500 Subject: [PATCH 2/9] Rename var in Settings.update() --- lbrynet/conf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lbrynet/conf.py b/lbrynet/conf.py index 180921b30..032529f27 100644 --- a/lbrynet/conf.py +++ b/lbrynet/conf.py @@ -67,8 +67,8 @@ class Settings(object): def get_dict(self): return {k: self[k] for k in self} - def update(self, other): - for k, v in other.iteritems(): + def update(self, updated_settings): + for k, v in updated_settings.iteritems(): try: self.__setitem__(k, v) except (KeyError, AssertionError): From 205a10bc8e296be636c8da527e42ef860934db6c Mon Sep 17 00:00:00 2001 From: Alex Liebowitz Date: Thu, 1 Dec 2016 08:53:58 -0500 Subject: [PATCH 3/9] Refactor how adjustable settings are pulled out of config.settings - Factor out this functionality into separate method - Change the set_settings() JSON-RPC method to use this (before, it was just returning all fields, which doesn't work anymore after the settings refactor) --- lbrynet/conf.py | 6 +++++- lbrynet/lbrynet_daemon/Daemon.py | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lbrynet/conf.py b/lbrynet/conf.py index 032529f27..1772317a2 100644 --- a/lbrynet/conf.py +++ b/lbrynet/conf.py @@ -270,6 +270,9 @@ class Config(DefaultSettings): def UI_ADDRESS(self): return "http://%s:%i" % (DEFAULT_SETTINGS.API_INTERFACE, self.api_port) + def get_adjustable_settings_dict(self): + return {opt: val for opt, val in self.get_dict().iteritems() if opt in ENVIRONMENT.original_schema} + def ensure_data_dir(self): # although there is a risk of a race condition here we don't # expect there to be multiple processes accessing this @@ -337,7 +340,8 @@ def load_settings(path): # or command line flag we don't want to persist it for future settings. def save_settings(path=None): path = path or settings.get_conf_filename() - to_save = {k: v for k, v in settings.__dict__.iteritems() if k in ADJUSTABLE_SETTINGS} + to_save = settings.get_adjustable_settings_dict() + ext = os.path.splitext(path)[1] encoder = settings_encoders.get(ext, False) assert encoder is not False, "Unknown settings format .%s" % ext diff --git a/lbrynet/lbrynet_daemon/Daemon.py b/lbrynet/lbrynet_daemon/Daemon.py index b3109c771..88f4a47ad 100644 --- a/lbrynet/lbrynet_daemon/Daemon.py +++ b/lbrynet/lbrynet_daemon/Daemon.py @@ -1255,12 +1255,12 @@ class Daemon(AuthJSONRPCServer): """ def _log_settings_change(): - log.info("Set daemon settings to " + json.dumps(conf.settings.configurable_settings)) + log.info("Set daemon settings to " + json.dumps(conf.settings.get_adjustable_settings_dict())) d = self._update_settings(p) d.addErrback(lambda err: log.info(err.getTraceback())) d.addCallback(lambda _: _log_settings_change()) - d.addCallback(lambda _: self._render_response(conf.settings.configurable_settings, OK_CODE)) + d.addCallback(lambda _: self._render_response(conf.settings.get_adjustable_settings_dict(), OK_CODE)) return d From 876cdce51f880d02d357f31629c1715d37fd4eec Mon Sep 17 00:00:00 2001 From: Alex Liebowitz Date: Thu, 1 Dec 2016 09:07:48 -0500 Subject: [PATCH 4/9] Add Config.get_dict() We don't want the version from DefaultSettings, so use super() to get the standard functionality from conf.Settings. --- lbrynet/conf.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lbrynet/conf.py b/lbrynet/conf.py index 1772317a2..4ad0cda1a 100644 --- a/lbrynet/conf.py +++ b/lbrynet/conf.py @@ -270,6 +270,9 @@ class Config(DefaultSettings): def UI_ADDRESS(self): return "http://%s:%i" % (DEFAULT_SETTINGS.API_INTERFACE, self.api_port) + def get_dict(self): + return {k: self[k] for k in self} + def get_adjustable_settings_dict(self): return {opt: val for opt, val in self.get_dict().iteritems() if opt in ENVIRONMENT.original_schema} From 7b1b7fbf131ccacd1e14411adc8936baaabd1626 Mon Sep 17 00:00:00 2001 From: Alex Liebowitz Date: Thu, 1 Dec 2016 09:22:47 -0500 Subject: [PATCH 5/9] Call conf.settings.get_dict() in jsonrpc_get_daemon_settings() --- lbrynet/lbrynet_daemon/Daemon.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lbrynet/lbrynet_daemon/Daemon.py b/lbrynet/lbrynet_daemon/Daemon.py index 88f4a47ad..0cb40c2e2 100644 --- a/lbrynet/lbrynet_daemon/Daemon.py +++ b/lbrynet/lbrynet_daemon/Daemon.py @@ -1234,7 +1234,7 @@ class Daemon(AuthJSONRPCServer): """ log.info("Get daemon settings") - return self._render_response(conf.settings.__dict__, OK_CODE) + return self._render_response(conf.settings.get_dict(), OK_CODE) @AuthJSONRPCServer.auth_required def jsonrpc_set_settings(self, p): From 157d6dca9340952dee63aa4c2d0254bf1f636c3f Mon Sep 17 00:00:00 2001 From: Alex Liebowitz Date: Thu, 1 Dec 2016 09:24:03 -0500 Subject: [PATCH 6/9] Save settings in conf.py --- lbrynet/lbrynet_daemon/Daemon.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lbrynet/lbrynet_daemon/Daemon.py b/lbrynet/lbrynet_daemon/Daemon.py index 0cb40c2e2..4bd20222f 100644 --- a/lbrynet/lbrynet_daemon/Daemon.py +++ b/lbrynet/lbrynet_daemon/Daemon.py @@ -665,6 +665,7 @@ class Daemon(AuthJSONRPCServer): except Exception as err: log.warning(err.message) log.warning("error converting setting '%s' to type %s", key, setting_type) + conf.save_settings() self.run_on_startup = conf.settings.run_on_startup self.data_rate = conf.settings.data_rate From 458561fe6fcf334a2acd48b6344e6178a7eec18f Mon Sep 17 00:00:00 2001 From: Alex Liebowitz Date: Thu, 1 Dec 2016 11:37:06 -0500 Subject: [PATCH 7/9] In AdjustableSettings, initialize all keys on init Settings.update() method expects keys to already be present, so load them all up front --- lbrynet/conf.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lbrynet/conf.py b/lbrynet/conf.py index 4ad0cda1a..bbcfd9306 100644 --- a/lbrynet/conf.py +++ b/lbrynet/conf.py @@ -185,6 +185,10 @@ class AdjustableSettings(Settings): """Settings that are allowed to be overriden by the user""" def __init__(self, environ=None): self.environ = environ or ENVIRONMENT + + for opt in self.environ.original_schema: + self.__dict__[opt] = self.environ(opt) + Settings.__init__(self) def __getattr__(self, attr): @@ -192,13 +196,6 @@ class AdjustableSettings(Settings): return self.environ(attr) raise AttributeError - def get_dict(self): - return { - name: self.environ(name) - for name in self.environ.original_schema - } - - class ApplicationSettings(Settings): """Settings that are constants and shouldn't be overriden""" def __init__(self): From 2bf098b5a314181af19a91ea3b6fd3cc0b173673 Mon Sep 17 00:00:00 2001 From: Job Evers-Meltzer Date: Wed, 30 Nov 2016 15:12:14 -0600 Subject: [PATCH 8/9] add tests for settings change --- tests/unit/test_conf.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tests/unit/test_conf.py diff --git a/tests/unit/test_conf.py b/tests/unit/test_conf.py new file mode 100644 index 000000000..1cd9a2e7f --- /dev/null +++ b/tests/unit/test_conf.py @@ -0,0 +1,36 @@ +import os + +from twisted.trial import unittest + +from lbrynet import conf + + +class SettingsTest(unittest.TestCase): + def setUp(self): + os.environ['LBRY_TEST'] = 'test_string' + + def tearDown(self): + del os.environ['LBRY_TEST'] + + def test_envvar_is_read(self): + env = conf.Env(test=(str, '')) + settings = conf.AdjustableSettings(env) + self.assertEqual('test_string', settings.test) + + def test_setting_can_be_overriden(self): + env = conf.Env(test=(str, '')) + settings = conf.AdjustableSettings(env) + settings.test = 'my_override' + self.assertEqual('my_override', settings.test) + + def test_setting_can_be_updated(self): + env = conf.Env(test=(str, '')) + settings = conf.AdjustableSettings(env) + settings.update({'test': 'my_update'}) + self.assertEqual('my_update', settings.test) + + def test_setting_is_in_dict(self): + env = conf.Env(test=(str, '')) + settings = conf.AdjustableSettings(env) + setting_dict = settings.get_dict() + self.assertEqual({'test': 'test_string'}, setting_dict) From 7d0e9f6ab97960a5c9525202a7136482c87fa02b Mon Sep 17 00:00:00 2001 From: Alex Liebowitz Date: Thu, 1 Dec 2016 23:51:55 -0500 Subject: [PATCH 9/9] Tweak logic in ApplicationSettings Use self.environ instead of ENVIRONMENT to avoid global reference --- lbrynet/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lbrynet/conf.py b/lbrynet/conf.py index bbcfd9306..c3c1910bb 100644 --- a/lbrynet/conf.py +++ b/lbrynet/conf.py @@ -271,7 +271,7 @@ class Config(DefaultSettings): return {k: self[k] for k in self} def get_adjustable_settings_dict(self): - return {opt: val for opt, val in self.get_dict().iteritems() if opt in ENVIRONMENT.original_schema} + return {opt: val for opt, val in self.get_dict().iteritems() if opt in self.environ.original_schema} def ensure_data_dir(self): # although there is a risk of a race condition here we don't