feat: add charge_timeout config option

ckanext/editable_config/config.py

@@ -5,6 +5,8 @@
 EXTRA_EDITABLE = "ckanext.editable_config.options.extra_editable"
 WHITELIST = "ckanext.editable_config.options.whitelist"
 BLACKLIST = "ckanext.editable_config.options.blacklist"
+CHARGE_TIMEOUT = "ckanext.editable_config.charge_timeout"
+DISABLE_CONFIG_TAB = "ckanext.editable_config.disable_admin_config_tab"
 
 
 def extra_editable() -> list[str]:
@@ -17,3 +19,11 @@ def whitelist() -> list[str]:
 
 def blacklist() -> list[str]:
     return tk.config[BLACKLIST]
+
+
+def charge_timeout() -> int:
+    return tk.config[CHARGE_TIMEOUT]
+
+
+def disable_admin_config_tab() -> bool:
+    return tk.config[DISABLE_CONFIG_TAB]

ckanext/editable_config/config_declaration.yaml

@@ -32,6 +32,21 @@ groups:
           to change the value of option mentioned here, even if it's
           `editable`.
 
+      - key: ckanext.editable_config.charge_timeout
+        type: int
+        default: 0
+        example: 10
+        description: |
+          Minimal number of seconds between two consequent change detection
+          cycles. Basically, if you set 60 as a value, plugin will check config
+          chnages once in a minute. In this way you can reduce number of DB
+          queries and avoid unnecesarry checks when static files are served by
+          CKAN.  Note, these queries are pretty fast(1ms), so you won't notice
+          any significant performance improvement by setting any non-zero value
+          here. And it means that any config overrides applied by API actions
+          may not be visible immediately - you'll have to wait `charge_timeout`
+          seconds in worst case.
+
       - key: ckanext.editable_config.disable_admin_config_tab
         type: bool
         example: true

ckanext/editable_config/helpers.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
-import ckan.plugins.toolkit as tk
+from . import config
+
 
 def editable_config_disable_admin_config_tab() -> bool:
-    return tk.config["ckanext.editable_config.disable_admin_config_tab"]
+    return config.disable_admin_config_tab()

ckanext/editable_config/model/option.py

@@ -23,13 +23,15 @@ class Option(tk.BaseModel):
 
     @classmethod
     def get(cls, key: str) -> Self | None:
+        """Search for option."""
         return cast(
             Self,
             model.Session.get(cls, key),
         )
 
     @classmethod
     def set(cls, key: str, value: Any) -> Self:
+        """Create/update an option."""
         option: Self
         safe_value = shared.value_as_string(key, value)
 
@@ -44,21 +46,36 @@ def set(cls, key: str, value: Any) -> Self:
         return option
 
     def touch(self):
+        """Update modification date of the option."""
         self.updated_at = datetime.utcnow()
 
     def revert(self):
+        """Swap current and previous values of the option."""
         self.value, self.prev_value = self.prev_value, self.value
         self.touch()
 
     def as_dict(self, context: types.Context) -> shared.OptionDict:
+        """Convert option object into form appropriate for API response."""
         return cast(shared.OptionDict, table_dictize(self, context))
 
     @classmethod
     def is_updated_since(cls, last_update: datetime | None) -> bool:
-        return model.Session.query(cls.updated_since(last_update).exists()).scalar()
+        """Check if there are any registered config overrides.
+
+        If optional `last_update` is provided, check for updates that were made
+        after this moment.
+        """
+        q: Any = cls.updated_since(last_update).exists()
+        return model.Session.query(q).scalar()
 
     @classmethod
     def updated_since(cls, last_update: datetime | None) -> types.Query[Self]:
+        """All overriden config options.
+
+        If optional `last_update` is provided, return only options that were
+        updated after this moment.
+
+        """
         q = model.Session.query(cls)
         if last_update:
             q = q.filter(cls.updated_at > last_update)

ckanext/editable_config/plugin.py

@@ -28,14 +28,14 @@ class EditableConfigPlugin(plugins.SingletonPlugin):
     plugins.implements(plugins.IConfigurer, inherit=True)
     plugins.implements(plugins.IConfigurable, inherit=True)
     plugins.implements(plugins.IMiddleware, inherit=True)
-    plugins.implements(plugins.IConfigDeclaration, inherit=True)
 
     _editable_config_enabled: bool = True
 
     # IMiddleware
     def make_middleware(self, app: types.CKANApp, config: CKANConfig) -> types.CKANApp:
         if self._editable_config_enabled:
             app.before_request(self._apply_overrides)
+
         return app
 
     def _apply_overrides(self):
@@ -74,6 +74,7 @@ def configure(self, config_: CKANConfig):
                 "editable_config disabled because of environemnt variable: %s",
                 ENVVAR_DISABLE,
             )
+            return
 
         inspector = sa.inspect(model.meta.engine)
         self._editable_config_enabled = inspector.has_table("editable_config_option")
@@ -91,8 +92,9 @@ def configure(self, config_: CKANConfig):
         }
 
         if problems := (set(legacy_modified) & editable):
-            if tk.config["ckanext.editable_config.convert_core_overrides"]:
+            if config.convert_core_overrides():
                 self._convert_core_overrides(problems)
+
             else:
                 log.warning(
                     "Modification via core AdminUI will cause undefined behavior: %s",
@@ -110,19 +112,18 @@ def _convert_core_overrides(self, names: Iterable[str]):
             return
 
         q = model.Session.query(model.SystemInfo).filter(
-            model.SystemInfo.key.in_(names)
+            model.SystemInfo.key.in_(names),
         )
-        options = {
-            op.key: op.value
-            for op in
-            q
-        }
+        options = {op.key: op.value for op in q}
 
         log.debug("Convert core overrides into editable config: %s", options)
-        change({"ignore_auth": True}, {
-            "apply": False,
-            "options": options,
-        })
+        change(
+            {"ignore_auth": True},
+            {
+                "apply": False,
+                "options": options,
+            },
+        )
 
         q.delete()
         model.Session.commit()

ckanext/editable_config/shared.py

@@ -10,9 +10,10 @@
 from ckan.cli import CKANConfigLoader
 from ckan.common import config_declaration as cd
 from ckan.config.declaration import Key
-from ckan.config.declaration.option import Option as DeclaredOption
 from ckan.plugins.core import plugins_update
 
+from . import config
+
 log = logging.getLogger(__name__)
 
 
@@ -24,18 +25,37 @@ class OptionDict(TypedDict):
 
 
 def value_as_string(key: str, value: Any) -> str:
+    """Convert the value into string using declared option rules."""
+    # TODO: Switch to `option.str_value(value)` once PR with this form is
+    # accepted and released.
     option = cd[Key.from_string(key)]
+    cls = type(option)
 
-    return DeclaredOption(value).set_validators(option.get_validators()).str_value()
+    return cls(value).set_validators(option.get_validators()).str_value()
 
 
 class _Updater:
+    # use the date of last update instead of mutex. In this way we can perform
+    # multiple simultaneous updates of the global config object, but it's ok,
+    # since config update is idempotent. More important, most of the time we
+    # won't spend extra ticks waiting for mutex acquire/release. Because config
+    # updates are relatively infrequent, it's better to do double-overrides
+    # once in a while instead of constantly waiting in mutex queue.
+
+    # TODO: write bencharks for mutex vs. last updated
+    # TODO: prove that race-condition is safe here
     _last_check: datetime.datetime | None
 
     def __init__(self):
         self._last_check = None
 
     def __call__(self, removed_keys: Collection[str] | None = None) -> 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.
+        """
         count = self._apply_changes()
         count += self._remove_keys(removed_keys)
 
@@ -45,11 +65,16 @@ def __call__(self, removed_keys: Collection[str] | None = None) -> int:
         return count
 
     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):
                 log.debug(
@@ -65,13 +90,15 @@ def _apply_changes(self) -> int:
         return count
 
     def _remove_keys(self, keys: Collection[str] | None) -> 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:
             if key in src_conf:
+                # switch to the literal value from the config file.
                 log.debug(
                     "Reset %s from %s to %s",
                     key,
@@ -81,13 +108,15 @@ def _remove_keys(self, keys: Collection[str] | None) -> int:
 
                 tk.config[key] = src_conf[key]
             elif key in tk.config:
+                # switch to the declared default value by removing option
                 log.debug(
                     "Remove %s with value %s",
                     key,
                     tk.config[key],
                 )
                 tk.config.pop(key)
             else:
+                # TODO: analize if it even possible to get here
                 log.warning("Attempt to reset unknown option %s", key)
                 continue
 

ckanext/editable_config/templates/admin/base.html

@@ -1,12 +1,13 @@
 {% ckan_extends %}
 
 {% block content_primary_nav %}
-
     {% if h.editable_config_disable_admin_config_tab() %}
         {{ h.build_nav_icon('admin.index', _('Sysadmins'), icon='gavel') }}
         {{ h.build_nav_icon('admin.trash', _('Trash'), icon='trash') }}
         {{ h.build_extra_admin_nav() if "build_extra_admin_nav" in h }}
+
     {% else %}
         {{ super() }}
+
     {% endif %}
 {% endblock %}

ckanext/editable_config/tests/conftest.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import contextlib
+from typing import Any
 
 import pytest
 from pytest_factoryboy import register
@@ -36,7 +37,7 @@ class Meta:
     @classmethod
     @contextlib.contextmanager
     def autoclean(cls, *args, **kwargs):
-        option = cls(*args, **kwargs)
+        option: dict[str, Any] = cls(*args, **kwargs)
         try:
             yield option
         finally:

ckanext/editable_config/tests/test_shared.py

@@ -5,7 +5,7 @@
 
 from ckan.tests.helpers import call_action
 
-from ckanext.editable_config import shared
+from ckanext.editable_config import config, shared
 from ckanext.editable_config.model import Option
 
 
@@ -26,7 +26,7 @@ def test_value_as_string():
 class TestUpdater:
     def test_apply_new_updates(self, faker, ckan_config, freezer, autoclean_option):
         """New updates are applied."""
-        freezer.move_to(timedelta(days=1))
+        freezer.move_to(timedelta(seconds=1))
         key = autoclean_option["key"]
         value = faker.sentence()
 
@@ -35,6 +35,21 @@ def test_apply_new_updates(self, faker, ckan_config, freezer, autoclean_option):
         assert shared.apply_config_overrides() == 1
         assert ckan_config[key] == value
 
+    @pytest.mark.ckan_config(config.CHARGE_TIMEOUT, 10)
+    def test_charge_timeout(self, faker, freezer, autoclean_option):
+        """New updates are applied."""
+        freezer.move_to(timedelta(seconds=5))
+
+        call_action(
+            "editable_config_change",
+            options={autoclean_option["key"]: faker.sentence()},
+            apply=False,
+        )
+        assert shared.apply_config_overrides() == 0
+
+        freezer.move_to(timedelta(seconds=6))
+        assert shared.apply_config_overrides() == 1
+
     def test_apply_old_updates(self, faker, ckan_config, freezer, autoclean_option):
         """Old updates are ignored."""
         freezer.move_to(timedelta(days=-1))