From d1b746a6cb04c87a351577b28be8752bd26d3c69 Mon Sep 17 00:00:00 2001 From: Jussi Vatjus-Anttila Date: Mon, 30 Oct 2023 12:30:24 +0200 Subject: [PATCH 1/4] add option to lock using nested objects using flatten object --- lockable/flatten.py | 23 +++++++++++++++++++++++ lockable/provider.py | 3 ++- tests/test_Lockable.py | 13 +++++++++++++ tests/test_Provider.py | 6 ++++++ tests/test_flatten.py | 36 ++++++++++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 lockable/flatten.py create mode 100644 tests/test_flatten.py diff --git a/lockable/flatten.py b/lockable/flatten.py new file mode 100644 index 0000000..bc872b5 --- /dev/null +++ b/lockable/flatten.py @@ -0,0 +1,23 @@ +def flatten_json(data: dict, parent_key='', sep='.') -> dict: + """ + Flatten a nested dictionary. + + Args: + - data (dict): The dictionary to flatten. + - parent_key (str, optional): The concatenated key from the parent(s). Defaults to ''. + - sep (str, optional): The separator to use between keys. Defaults to '.'. + + Returns: + - dict: The flattened dictionary. + """ + items = {} + for k, v in data.items(): + new_key = f"{parent_key}{sep}{k}" if parent_key else k + if isinstance(v, dict): + items.update(flatten_json(v, new_key, sep=sep)) + else: + items[new_key] = v + return items + +def flatten_list(array: list) -> list: + return list(map(flatten_json, array)) diff --git a/lockable/provider.py b/lockable/provider.py index 3013b2b..34ccd55 100644 --- a/lockable/provider.py +++ b/lockable/provider.py @@ -6,6 +6,7 @@ from pydash import filter_, count_by +from lockable.flatten import flatten_list from lockable.logger import get_logger MODULE_LOGGER = get_logger() @@ -26,7 +27,7 @@ def __init__(self, uri: typing.Union[str, list]): @property def data(self) -> list: """ Get resources list """ - return self._resources + return flatten_list(self._resources) @abstractmethod def reload(self) -> None: # pragma: no cover diff --git a/tests/test_Lockable.py b/tests/test_Lockable.py index 15257d8..383feab 100644 --- a/tests/test_Lockable.py +++ b/tests/test_Lockable.py @@ -274,3 +274,16 @@ def test_lock_many_existing_allocation(self): self.assertTrue(end - start < 2 and end - start > 1) self.assertTrue(os.path.exists(os.path.join(tmpdirname, 'a.pid'))) self.assertFalse(os.path.exists(os.path.join(tmpdirname, 'b.pid'))) + + def test_lock_nested(self): + with TemporaryDirectory() as tmpdirname: + resources = [ + {"id": "a", "hostname": "myhost", "online": True, "a": 2}, + {"id": "b", "hostname": "myhost", "online": True, "a": {"b": 2}} + ] + + lockable = Lockable(hostname='myhost', resource_list=resources, lock_folder=tmpdirname) + resource = lockable.lock({"a.b": 2}) + self.assertEqual(resource.resource_id, 'b') + self.assertEqual(resource.resource_info, + {"id": "b", "hostname": "myhost", "online": True, "a.b": 2}) diff --git a/tests/test_Provider.py b/tests/test_Provider.py index b8479d9..c28089b 100644 --- a/tests/test_Provider.py +++ b/tests/test_Provider.py @@ -163,3 +163,9 @@ def test_provider_http_no_response(self): ProviderHttp.BACKOFF_FACTOR = 0 with self.assertRaises(ProviderError): create_provider('http://localhost/resource') + + def test_provider_nested_resources(self): + nested_resource = {"id": 12, "a": {"b": 2}} + flatten_resource = {"a.b": 2, "id": 12} + provider = create_provider([nested_resource]) + self.assertEqual([flatten_resource], provider.data) diff --git a/tests/test_flatten.py b/tests/test_flatten.py new file mode 100644 index 0000000..5098d4c --- /dev/null +++ b/tests/test_flatten.py @@ -0,0 +1,36 @@ +import unittest + +from lockable.flatten import flatten_json, flatten_list + + +class TestFlattenJson(unittest.TestCase): + + def test_basic_flattening(self): + data = {"a": {"b": 1}} + result = flatten_json(data) + self.assertEqual(result, {"a.b": 1}) + + def test_multiple_level_flattening(self): + data = {"a": {"b": {"c": 2}}} + result = flatten_json(data) + self.assertEqual(result, {"a.b.c": 2}) + + def test_mixed_flattening(self): + data = {"a": {"b": 1, "c": {"d": 2, "e": {"f": 3}}}, "g": 4} + result = flatten_json(data) + self.assertEqual(result, {'a.b': 1, 'a.c.d': 2, 'a.c.e.f': 3, 'g': 4}) + + def test_empty_data(self): + data = {} + result = flatten_json(data) + self.assertEqual(result, {}) + + def test_non_nested_data(self): + data = {"a": 1, "b": 2} + result = flatten_json(data) + self.assertEqual(result, data) + + def test_flatten_list_mixed(self): + data = [{"a": 1, "b": 2}, {"a": {"b": {"c": 2}}}] + result = flatten_list(data) + self.assertEqual(result, [{'a': 1, 'b': 2},{ 'a.b.c': 2}]) From 6c0648f3548bbb6377a0d2d163ff3d30c5bbc1f8 Mon Sep 17 00:00:00 2001 From: Jussi Vatjus-Anttila Date: Mon, 30 Oct 2023 12:40:25 +0200 Subject: [PATCH 2/4] update pylint issues --- .pylintrc | 93 +-------------------------------------------- lockable/flatten.py | 17 ++++++--- 2 files changed, 13 insertions(+), 97 deletions(-) diff --git a/.pylintrc b/.pylintrc index 4fc5e6a..81dd547 100644 --- a/.pylintrc +++ b/.pylintrc @@ -60,85 +60,7 @@ confidence= # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use "--disable=all --enable=classes # --disable=W". -disable=print-statement, - parameter-unpacking, - unpacking-in-except, - old-raise-syntax, - backtick, - long-suffix, - old-ne-operator, - old-octal-literal, - import-star-module-level, - non-ascii-bytes-literal, - raw-checker-failed, - bad-inline-option, - locally-disabled, - file-ignored, - suppressed-message, - useless-suppression, - deprecated-pragma, - use-symbolic-message-instead, - apply-builtin, - basestring-builtin, - buffer-builtin, - cmp-builtin, - coerce-builtin, - execfile-builtin, - file-builtin, - long-builtin, - raw_input-builtin, - reduce-builtin, - standarderror-builtin, - unicode-builtin, - xrange-builtin, - coerce-method, - delslice-method, - getslice-method, - setslice-method, - no-absolute-import, - old-division, - dict-iter-method, - dict-view-method, - next-method-called, - metaclass-assignment, - indexing-exception, - raising-string, - reload-builtin, - oct-method, - hex-method, - nonzero-method, - cmp-method, - input-builtin, - round-builtin, - intern-builtin, - unichr-builtin, - map-builtin-not-iterating, - zip-builtin-not-iterating, - range-builtin-not-iterating, - filter-builtin-not-iterating, - using-cmp-argument, - eq-without-hash, - div-method, - idiv-method, - rdiv-method, - exception-message-attribute, - invalid-str-codec, - sys-max-int, - bad-python3-import, - deprecated-string-function, - deprecated-str-translate-call, - deprecated-itertools-function, - deprecated-types-field, - next-method-defined, - dict-items-not-iterating, - dict-keys-not-iterating, - dict-values-not-iterating, - deprecated-operator-function, - deprecated-urllib-function, - xreadlines-attribute, - deprecated-sys-function, - exception-escape, - comprehension-escape +disable= # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option @@ -329,12 +251,6 @@ max-line-length=100 # Maximum number of lines in a module. max-module-lines=1000 -# List of optional constructs for which whitespace checking is disabled. `dict- -# separator` is used to allow tabulation in dicts, etc.: {1 : 1,\n222: 2}. -# `trailing-comma` allows a space between comma and closing bracket: (a, ). -# `empty-line` allows space-only lines. -no-space-check=trailing-comma, - dict-separator # Allow the body of a class to be on the same line as the declaration if body # contains single statement. @@ -572,10 +488,3 @@ max-statements=50 # Minimum number of public methods for a class (see R0903). min-public-methods=2 - -[EXCEPTIONS] - -# Exceptions that will emit a warning when being caught. Defaults to -# "BaseException, Exception". -overgeneral-exceptions=BaseException, - Exception diff --git a/lockable/flatten.py b/lockable/flatten.py index bc872b5..f423495 100644 --- a/lockable/flatten.py +++ b/lockable/flatten.py @@ -1,3 +1,8 @@ +""" +Helper functions for flattening nested dictionaries. +""" + + def flatten_json(data: dict, parent_key='', sep='.') -> dict: """ Flatten a nested dictionary. @@ -11,13 +16,15 @@ def flatten_json(data: dict, parent_key='', sep='.') -> dict: - dict: The flattened dictionary. """ items = {} - for k, v in data.items(): - new_key = f"{parent_key}{sep}{k}" if parent_key else k - if isinstance(v, dict): - items.update(flatten_json(v, new_key, sep=sep)) + for key, value in data.items(): + new_key = f"{parent_key}{sep}{key}" if parent_key else key + if isinstance(value, dict): + items.update(flatten_json(value, new_key, sep=sep)) else: - items[new_key] = v + items[new_key] = value return items + def flatten_list(array: list) -> list: + """ Flatten a list of dictionaries """ return list(map(flatten_json, array)) From 4b2b927ad250a469385155b4f1d41bde2eeb3ed8 Mon Sep 17 00:00:00 2001 From: Jussi Vatjus-Anttila Date: Mon, 30 Oct 2023 15:28:52 +0200 Subject: [PATCH 3/4] ensure we never overwrite values --- lockable/flatten.py | 2 ++ lockable/provider_file.py | 2 +- tests/test_flatten.py | 7 ++++++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lockable/flatten.py b/lockable/flatten.py index f423495..96be2ee 100644 --- a/lockable/flatten.py +++ b/lockable/flatten.py @@ -21,6 +21,8 @@ def flatten_json(data: dict, parent_key='', sep='.') -> dict: if isinstance(value, dict): items.update(flatten_json(value, new_key, sep=sep)) else: + # ensure that the key doesn't overwrite an existing key + assert new_key not in items, f"Key {new_key} already exists in items" items[new_key] = value return items diff --git a/lockable/provider_file.py b/lockable/provider_file.py index a36e757..d754070 100644 --- a/lockable/provider_file.py +++ b/lockable/provider_file.py @@ -17,7 +17,7 @@ def __init__(self, uri: str): ProviderFile constructor :param uri: file path """ - MODULE_LOGGER.debug('Creating ProviderFile using %s', uri) + MODULE_LOGGER.debug(f'Creating ProviderFile using {uri}') self._resource_list_file_mtime = None super().__init__(uri) diff --git a/tests/test_flatten.py b/tests/test_flatten.py index 5098d4c..0ed14a8 100644 --- a/tests/test_flatten.py +++ b/tests/test_flatten.py @@ -33,4 +33,9 @@ def test_non_nested_data(self): def test_flatten_list_mixed(self): data = [{"a": 1, "b": 2}, {"a": {"b": {"c": 2}}}] result = flatten_list(data) - self.assertEqual(result, [{'a': 1, 'b': 2},{ 'a.b.c': 2}]) + self.assertEqual(result, [{'a': 1, 'b': 2}, {'a.b.c': 2}]) + + def test_raise_if_overlapping_subfield(self): + data = {"a": {"b": 1}, "a.b": 2} + with self.assertRaises(AssertionError): + flatten_json(data) From 085df7f8784625f390e3851217fda7c736330f67 Mon Sep 17 00:00:00 2001 From: Jussi Vatjus-Anttila Date: Mon, 30 Oct 2023 15:38:10 +0200 Subject: [PATCH 4/4] revert fstring that was not allowed --- lockable/provider_file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lockable/provider_file.py b/lockable/provider_file.py index d754070..a36e757 100644 --- a/lockable/provider_file.py +++ b/lockable/provider_file.py @@ -17,7 +17,7 @@ def __init__(self, uri: str): ProviderFile constructor :param uri: file path """ - MODULE_LOGGER.debug(f'Creating ProviderFile using {uri}') + MODULE_LOGGER.debug('Creating ProviderFile using %s', uri) self._resource_list_file_mtime = None super().__init__(uri)