From 98caac826984fc6b9581b20f649565f1502fad7b Mon Sep 17 00:00:00 2001 From: Anatoly Scherbakov Date: Fri, 21 Jun 2019 18:01:48 +0700 Subject: [PATCH 1/5] Move parser configuration out of FileConfig.__init__() --- datadotworld/config.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/datadotworld/config.py b/datadotworld/config.py index c24dec4..b63fae8 100644 --- a/datadotworld/config.py +++ b/datadotworld/config.py @@ -88,6 +88,7 @@ class FileConfig(DefaultConfig): :param profile: Name of configuration profile. :type profile: str """ + _config_parser = None def __init__(self, profile='default', **kwargs): super(FileConfig, self).__init__() @@ -95,9 +96,17 @@ def __init__(self, profile='default', **kwargs): # Overrides, for testing self._config_file_path = path.expanduser( kwargs.get('config_file_path', '~/.dw/config')) - legacy_file_path = path.expanduser( + self.legacy_file_path = path.expanduser( kwargs.get('legacy_file_path', '~/.data.world')) + self._profile = profile + self._section = (profile + if profile.lower() != configparser.DEFAULTSECT.lower() + else configparser.DEFAULTSECT) + + self.configure_config_parser() + + def configure_config_parser(self): if not path.isdir(path.dirname(self._config_file_path)): os.makedirs(path.dirname(self._config_file_path)) @@ -108,15 +117,10 @@ def __init__(self, profile='default', **kwargs): self._config_parser.read_file(open(self._config_file_path)) if self.__migrate_invalid_defaults(self._config_parser) > 0: self.save() - elif path.isfile(legacy_file_path): - self._config_parser = self.__migrate_config(legacy_file_path) + elif path.isfile(self.legacy_file_path): + self._config_parser = self.__migrate_config(self.legacy_file_path) self.save() - self._profile = profile - self._section = (profile - if profile.lower() != configparser.DEFAULTSECT.lower() - else configparser.DEFAULTSECT) - if not path.isdir(path.dirname(self.cache_dir)): os.makedirs(path.dirname(self.cache_dir)) From 99c5b74bd0b41289ed441d18f16c2b47fd621fc4 Mon Sep 17 00:00:00 2001 From: Anatoly Scherbakov Date: Fri, 21 Jun 2019 18:52:56 +0700 Subject: [PATCH 2/5] lazy configuration parser for FileConfig --- datadotworld/config.py | 22 +++++++++++++++------- tests/datadotworld/test_config.py | 7 +++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/datadotworld/config.py b/datadotworld/config.py index b63fae8..37d96a9 100644 --- a/datadotworld/config.py +++ b/datadotworld/config.py @@ -104,7 +104,12 @@ def __init__(self, profile='default', **kwargs): if profile.lower() != configparser.DEFAULTSECT.lower() else configparser.DEFAULTSECT) - self.configure_config_parser() + @property + def config_parser(self): + if self._config_parser is None: + self.configure_config_parser() + + return self._config_parser def configure_config_parser(self): if not path.isdir(path.dirname(self._config_file_path)): @@ -127,7 +132,7 @@ def configure_config_parser(self): @property def auth_token(self): self.__validate_config() - return self._config_parser.get(self._section, 'auth_token') + return self.config_parser.get(self._section, 'auth_token') @auth_token.setter def auth_token(self, auth_token): @@ -137,22 +142,25 @@ def auth_token(self, auth_token): """ if (self._section != configparser.DEFAULTSECT and - not self._config_parser.has_section(self._section)): - self._config_parser.add_section(self._section) - self._config_parser.set(self._section, 'auth_token', auth_token) + not self.config_parser.has_section(self._section)): + self.config_parser.add_section(self._section) + self.config_parser.set(self._section, 'auth_token', auth_token) def save(self): """Persist config changes""" with open(self._config_file_path, 'w') as file: - self._config_parser.write(file) + self.config_parser.write(file) def __validate_config(self): + config_parser = self.config_parser + if not path.isfile(self._config_file_path): raise RuntimeError( 'Configuration file not found at {}.' 'To fix this issue, run dw configure'.format( self._config_file_path)) - if not self._config_parser.has_option(self._section, 'auth_token'): + + if not config_parser.has_option(self._section, 'auth_token'): raise RuntimeError( 'The {0} profile is not properly configured. ' 'To fix this issue, run dw -p {0} configure'.format( diff --git a/tests/datadotworld/test_config.py b/tests/datadotworld/test_config.py index 645a7bf..9216cce 100644 --- a/tests/datadotworld/test_config.py +++ b/tests/datadotworld/test_config.py @@ -191,6 +191,13 @@ def test_save_overwrite(self, config_file_path): config_reloaded = FileConfig(config_file_path=config_file_path) assert_that(config_reloaded.auth_token, equal_to('newtoken')) + @pytest.mark.usefixtures('config_directory', 'default_config_file') + def test_invalid_config_file_path(self, config_file_path): + config = FileConfig(config_file_path='/foo/bar/baz/') + + with pytest.raises(PermissionError): + config.configure_config_parser() + class TestChainedConfig: @pytest.fixture() From 7505af1b0fb7c03a68a26f45d38b75622fb6b362 Mon Sep 17 00:00:00 2001 From: Anatoly Scherbakov Date: Fri, 21 Jun 2019 19:30:20 +0700 Subject: [PATCH 3/5] debug --- datadotworld/config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/datadotworld/config.py b/datadotworld/config.py index 37d96a9..7a329d7 100644 --- a/datadotworld/config.py +++ b/datadotworld/config.py @@ -237,6 +237,7 @@ def _first_not_none(seq, supplier_func): """ for i in seq: obj = supplier_func(i) + print('{} => {}'.format(i, obj)) if obj is not None: return obj From 693e6bffaf29deea646540ded3a5f65a9c189505 Mon Sep 17 00:00:00 2001 From: Anatoly Scherbakov Date: Fri, 21 Jun 2019 19:46:56 +0700 Subject: [PATCH 4/5] remove print and add some cosmetics --- datadotworld/config.py | 10 +++++++--- tests/datadotworld/test_config.py | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/datadotworld/config.py b/datadotworld/config.py index 7a329d7..223fe57 100644 --- a/datadotworld/config.py +++ b/datadotworld/config.py @@ -107,11 +107,12 @@ def __init__(self, profile='default', **kwargs): @property def config_parser(self): if self._config_parser is None: - self.configure_config_parser() + self._config_parser = self.get_config_parser() return self._config_parser - def configure_config_parser(self): + def get_config_parser(self): + # FIXME this should probably be a pure function if not path.isdir(path.dirname(self._config_file_path)): os.makedirs(path.dirname(self._config_file_path)) @@ -120,8 +121,10 @@ def configure_config_parser(self): if path.isfile(self._config_file_path): self._config_parser.read_file(open(self._config_file_path)) + if self.__migrate_invalid_defaults(self._config_parser) > 0: self.save() + elif path.isfile(self.legacy_file_path): self._config_parser = self.__migrate_config(self.legacy_file_path) self.save() @@ -129,6 +132,8 @@ def configure_config_parser(self): if not path.isdir(path.dirname(self.cache_dir)): os.makedirs(path.dirname(self.cache_dir)) + return self._config_parser + @property def auth_token(self): self.__validate_config() @@ -237,7 +242,6 @@ def _first_not_none(seq, supplier_func): """ for i in seq: obj = supplier_func(i) - print('{} => {}'.format(i, obj)) if obj is not None: return obj diff --git a/tests/datadotworld/test_config.py b/tests/datadotworld/test_config.py index 9216cce..2542152 100644 --- a/tests/datadotworld/test_config.py +++ b/tests/datadotworld/test_config.py @@ -196,7 +196,7 @@ def test_invalid_config_file_path(self, config_file_path): config = FileConfig(config_file_path='/foo/bar/baz/') with pytest.raises(PermissionError): - config.configure_config_parser() + config.get_config_parser() class TestChainedConfig: From b3a115c769e725984a2d38c202dfe3a205c8d354 Mon Sep 17 00:00:00 2001 From: Anatoly Scherbakov Date: Fri, 21 Jun 2019 20:12:35 +0700 Subject: [PATCH 5/5] cache_dir should now be overridable --- datadotworld/config.py | 4 +++- tests/datadotworld/test_config.py | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/datadotworld/config.py b/datadotworld/config.py index 223fe57..1230868 100644 --- a/datadotworld/config.py +++ b/datadotworld/config.py @@ -45,7 +45,7 @@ class DefaultConfig(object): def __init__(self): self._auth_token = None self._tmp_dir = path.expanduser(tempfile.gettempdir()) - self._cache_dir = path.expanduser('~/.dw/cache') + self._cache_dir = None @property def auth_token(self): @@ -104,6 +104,8 @@ def __init__(self, profile='default', **kwargs): if profile.lower() != configparser.DEFAULTSECT.lower() else configparser.DEFAULTSECT) + self._cache_dir = path.expanduser('~/.dw/cache') + @property def config_parser(self): if self._config_parser is None: diff --git a/tests/datadotworld/test_config.py b/tests/datadotworld/test_config.py index 2542152..7411100 100644 --- a/tests/datadotworld/test_config.py +++ b/tests/datadotworld/test_config.py @@ -60,7 +60,7 @@ def test_auth_token(self): def test_cache_dir(self): assert_that(DefaultConfig().cache_dir, - equal_to(path.expanduser('~/.dw/cache'))) + equal_to(None)) def test_tmp_dir(self): assert_that(DefaultConfig().tmp_dir, @@ -75,7 +75,7 @@ def test_auth_token(self): def test_cache_dir(self): config = InlineConfig('inline_token') assert_that(config.cache_dir, - equal_to(path.expanduser('~/.dw/cache'))) + equal_to(None)) def test_tmp_dir(self): config = InlineConfig('inline_token') @@ -198,6 +198,11 @@ def test_invalid_config_file_path(self, config_file_path): with pytest.raises(PermissionError): config.get_config_parser() + def test_cache_dir(self): + config = FileConfig() + assert_that(config.cache_dir, + equal_to(path.expanduser('~/.dw/cache'))) + class TestChainedConfig: @pytest.fixture()