From aa10075d6e8b506d1ff6f6595789d769f57a226f Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 8 May 2019 15:04:34 -0700 Subject: [PATCH 1/3] Expose loaded config files, make load idempotent This change enables applications to implement periodic reloads of updated configuration files without requiring restarts. This is intended for long-running, service-oriented, applications that cannot afford to be shutdown yet may need to have their configurations updated. This change exposes the _loaded_config_files list on the application class instance via a property. This was done so as to not allow side-affects to creep in from clients. More importantly, is the prevention of duplicate config file entries that can occur when load_config_file is called multiple times. This i enables the method's idempotency - otherwise the list of loaded config files increased on each (periodic) call. --- traitlets/config/application.py | 8 ++++- traitlets/config/tests/test_application.py | 42 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/traitlets/config/application.py b/traitlets/config/application.py index b439822c..01bd7149 100644 --- a/traitlets/config/application.py +++ b/traitlets/config/application.py @@ -754,6 +754,11 @@ def _load_config_files(cls, basefilename, path=None, log=None, raise_config_file loaded.append(config) filenames.append(loader.full_filename) + @property + def loaded_config_files(self): + """Currently loaded configuration files""" + return self._loaded_config_files.copy() + @catch_config_error def load_config_file(self, filename, path=None): """Load config files by filename and path.""" @@ -763,7 +768,8 @@ def load_config_file(self, filename, path=None): raise_config_file_errors=self.raise_config_file_errors, ): new_config.merge(config) - self._loaded_config_files.append(filename) + if filename not in self._loaded_config_files: # only add if not present (support reloads) + self._loaded_config_files.append(filename) # add self.cli_config to preserve CLI config priority new_config.merge(self.cli_config) self.update_config(new_config) diff --git a/traitlets/config/tests/test_application.py b/traitlets/config/tests/test_application.py index bf66eb71..30b89178 100644 --- a/traitlets/config/tests/test_application.py +++ b/traitlets/config/tests/test_application.py @@ -547,6 +547,48 @@ def test_subcommands_instanciation(self): self.assertIs(app.subapp.parent, app) self.assertIs(app.subapp.subapp.parent, app.subapp) # Set by factory. + def test_loaded_config_files(self): + app = MyApp() + app.log = logging.getLogger() + name = 'config.py' + with TemporaryDirectory('_1') as td1: + config_file = pjoin(td1, name) + with open(config_file, 'w') as f: + f.writelines([ + "c.MyApp.running = True\n" + ]) + + app.load_config_file(name, path=[td1]) + self.assertEqual(len(app.loaded_config_files), 1) + self.assertEquals(app.loaded_config_files[0], config_file) + + app.start() + self.assertEqual(app.running, True) + + # emulate an app that allows dynamic updates and update config file + with open(config_file, 'w') as f: + f.writelines([ + "c.MyApp.running = False\n" + ]) + + # reload and verify update, and that loaded_configs was not increased + app.load_config_file(name, path=[td1]) + self.assertEqual(len(app.loaded_config_files), 1) + self.assertEqual(app.running, False) + + # Attempt to update, ensure error... + with self.assertRaises(AttributeError): + app.loaded_config_files = "/foo" + + # ensure it can't be udpated via append + app.loaded_config_files.append("/bar") + self.assertEqual(len(app.loaded_config_files), 1) + + # repeat to ensure no unexpected changes occurred + app.load_config_file(name, path=[td1]) + self.assertEqual(len(app.loaded_config_files), 1) + self.assertEqual(app.running, False) + class Root(Application): subcommands = { From f53f209b558b84c2eff256429351e02d93c076fe Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Wed, 8 May 2019 16:53:54 -0700 Subject: [PATCH 2/3] Use more compatible form of copy for lists --- traitlets/config/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traitlets/config/application.py b/traitlets/config/application.py index 01bd7149..c45d701a 100644 --- a/traitlets/config/application.py +++ b/traitlets/config/application.py @@ -757,7 +757,7 @@ def _load_config_files(cls, basefilename, path=None, log=None, raise_config_file @property def loaded_config_files(self): """Currently loaded configuration files""" - return self._loaded_config_files.copy() + return self._loaded_config_files[:] @catch_config_error def load_config_file(self, filename, path=None): From 11b5610f5b68592e63377fcb361cf59357df798f Mon Sep 17 00:00:00 2001 From: Kevin Bates Date: Thu, 30 May 2019 06:47:31 -0700 Subject: [PATCH 3/3] make comment more clear --- traitlets/config/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traitlets/config/application.py b/traitlets/config/application.py index c45d701a..e7c28e98 100644 --- a/traitlets/config/application.py +++ b/traitlets/config/application.py @@ -768,7 +768,7 @@ def load_config_file(self, filename, path=None): raise_config_file_errors=self.raise_config_file_errors, ): new_config.merge(config) - if filename not in self._loaded_config_files: # only add if not present (support reloads) + if filename not in self._loaded_config_files: # only add to list of loaded files if not previously loaded self._loaded_config_files.append(filename) # add self.cli_config to preserve CLI config priority new_config.merge(self.cli_config)