From 9f4e830af4f1c75401388da6071481275889d615 Mon Sep 17 00:00:00 2001 From: Sergey Motornyuk Date: Fri, 7 Jun 2024 17:42:26 +0300 Subject: [PATCH] fix: reset does not work in multi-process mode --- ckanext/editable_config/logic/action.py | 6 +-- ckanext/editable_config/shared.py | 37 ++++++++++++------- .../tests/logic/test_action.py | 2 +- ckanext/editable_config/tests/test_shared.py | 32 +++++++--------- 4 files changed, 42 insertions(+), 35 deletions(-) diff --git a/ckanext/editable_config/logic/action.py b/ckanext/editable_config/logic/action.py index d770972..550415e 100644 --- a/ckanext/editable_config/logic/action.py +++ b/ckanext/editable_config/logic/action.py @@ -101,7 +101,7 @@ def editable_config_update( ) if data_dict["apply"]: - shared.apply_config_overrides(removed_keys=result["reset"]) + shared.apply_config_overrides() return result @@ -225,7 +225,7 @@ def editable_config_reset( sess.commit() if data_dict["apply"]: - shared.apply_config_overrides(removed_keys=result) + shared.apply_config_overrides() return result @@ -236,6 +236,6 @@ def editable_config_apply( data_dict: dict[str, Any], ) -> dict[str, Any]: tk.check_access("editable_config_apply", context, data_dict) - count = shared.apply_config_overrides(removed_keys=data_dict["removed_keys"]) + count = shared.apply_config_overrides() return {"count": count} diff --git a/ckanext/editable_config/shared.py b/ckanext/editable_config/shared.py index 0587d33..38f2ddb 100644 --- a/ckanext/editable_config/shared.py +++ b/ckanext/editable_config/shared.py @@ -2,7 +2,7 @@ import datetime import logging -from typing import Any, Collection, Iterable +from typing import Any, Iterable import sqlalchemy as sa from typing_extensions import TypedDict @@ -137,6 +137,7 @@ class _Updater: # TODO: write bencharks for mutex vs. last updated # TODO: prove that race-condition is safe here _last_check: datetime.datetime | None + _active_overrides: set[str] @property def last_check(self): @@ -144,17 +145,25 @@ def last_check(self): def __init__(self): self._last_check = None + self._active_overrides = set() - def __call__(self, removed_keys: Collection[str] | None = None) -> int: + def __call__(self) -> int: """Override changed config options and remove options that do not require customization. Reckon total number of modifications and reload plugins if any change detected. """ + now = datetime.datetime.utcnow() + + charge_timeout = datetime.timedelta(seconds=config.charge_timeout()) + if self._last_check and now - self._last_check < charge_timeout: + return 0 + count = self._apply_changes() - count += self._remove_keys(removed_keys) + count += self._remove_keys() + self._last_check = now if count: plugins_update() @@ -164,13 +173,8 @@ def _apply_changes(self) -> int: """Override config options that were updated since last check.""" from ckanext.editable_config.model import Option - now = datetime.datetime.utcnow() count = 0 - charge_timeout = datetime.timedelta(seconds=config.charge_timeout()) - if self._last_check and now - self._last_check < charge_timeout: - return count - if Option.is_updated_since(self._last_check): for option in Option.updated_since(self._last_check): if not is_editable(option.key): @@ -189,17 +193,23 @@ def _apply_changes(self) -> int: tk.config[option.key] = option.value count += 1 - self._last_check = now return count - def _remove_keys(self, keys: Collection[str] | None) -> int: + def _remove_keys(self) -> int: """Restore original value(using config file) for specified options.""" count = 0 - if not keys: - return count src_conf = CKANConfigLoader(tk.config["__file__"]).get_config() - for key in keys: + + try: + editable = tk.get_action("editable_config_list")({"ignore_auth": True}, {}) + except KeyError: + log.debug("Do not check removed overrides because plugin is not loaded yet") + return 0 + + current_overrides = {k for k, v in editable.items() if v["option"]} + + for key in self._active_overrides - current_overrides: if key in src_conf: # switch to the literal value from the config file. log.debug( @@ -225,6 +235,7 @@ def _remove_keys(self, keys: Collection[str] | None) -> int: count += 1 + self._active_overrides = current_overrides return count diff --git a/ckanext/editable_config/tests/logic/test_action.py b/ckanext/editable_config/tests/logic/test_action.py index e3fe304..74c3409 100644 --- a/ckanext/editable_config/tests/logic/test_action.py +++ b/ckanext/editable_config/tests/logic/test_action.py @@ -40,7 +40,7 @@ def test_one_per_group(self, faker, option_factory, ckan_config): apply=False, ) - assert shared.apply_config_overrides(removed_keys=result["reset"]) == 3 + assert shared.apply_config_overrides() == 3 assert ( ckan_config["ckan.site_title"] diff --git a/ckanext/editable_config/tests/test_shared.py b/ckanext/editable_config/tests/test_shared.py index 454b207..ef88de8 100644 --- a/ckanext/editable_config/tests/test_shared.py +++ b/ckanext/editable_config/tests/test_shared.py @@ -59,27 +59,23 @@ def test_apply_old_updates(self, faker, ckan_config, freezer, autoclean_option): assert shared.apply_config_overrides() == 0 assert ckan_config[key] != value - def test_reset_configured_option(self, ckan_config, autoclean_option): + @pytest.mark.usefixtures("with_autoclean") + def test_reset_configured_option(self, ckan_config, option_factory, faker): """Config overrides for option in config file can are removed.""" - key = autoclean_option["key"] - assert shared.apply_config_overrides(removed_keys=[key]) == 1 - assert ckan_config[key] != autoclean_option["value"] + key = "ckan.site_title" + option = option_factory(key=key, value=faker.sentence()) + assert shared.apply_config_overrides() == 0 + + call_action("editable_config_reset", keys=[key], apply=False) + assert shared.apply_config_overrides() == 1 + assert ckan_config[key] != option["value"] def test_reset_added_option(self, faker, ckan_config, option_factory): """Config overrides for option not available in the config file are removed.""" key = "ckan.site_intro_text" + option = option_factory(key=key, value=faker.sentence()) + assert shared.apply_config_overrides() == 0 - with option_factory.autoclean(key=key, value=faker.sentence()) as option: - assert shared.apply_config_overrides(removed_keys=[key]) == 1 - assert ckan_config[key] != option["value"] - - @pytest.mark.ckan_config("ckan.site_title", "hello world") - def test_reset_original_option(self, faker, ckan_config): - """Config options can be restored to the state of config file even when - changed elsewhere. - - """ - key = "ckan.site_title" - assert ckan_config[key] == "hello world" - assert shared.apply_config_overrides(removed_keys=[key]) == 1 - assert ckan_config[key] != "hello world" + call_action("editable_config_reset", keys=[key], apply=False) + assert shared.apply_config_overrides() == 1 + assert ckan_config[key] != option["value"]