Merge pull request #327 from johannaengland/optimization/reload-pollf…

…ile-on-nmtime-changed

Only load and parse pollfile if it has changed since last loading

changelog.d/282.added.md

@@ -0,0 +1 @@
+Only load and parse pollfile if it has been changed since last load

src/zino/scheduler.py

@@ -1,6 +1,7 @@
 import asyncio
 import logging
 import operator
+import pathlib
 from datetime import datetime, timedelta
 from typing import Sequence, Set, Tuple
 
@@ -41,13 +42,23 @@ def get_scheduler() -> AsyncIOScheduler:
 
 @log_time_spent()
 def load_polldevs(polldevs_conf: str) -> Tuple[Set, Set, Set, dict[str, str]]:
-    """Loads pollfile into process state.
+    """Loads pollfile into process state if it was changed since the last time it
+    was loaded
 
     :returns: A tuple of (new_devices, deleted_devices, changed_devices, default_settings)
     """
+    try:
+        modified_time = pathlib.Path(polldevs_conf).stat().st_mtime
+    except OSError as error:
+        _log.error(error)
+        return set(), set(), set(), dict()
+
+    if modified_time == state.pollfile_mtime:
+        return set(), set(), set(), dict()
+
     try:
         devices, defaults = read_polldevs(polldevs_conf)
-    except InvalidConfiguration as error:
+    except (InvalidConfiguration, OSError) as error:
         _log.error(error)
         return set(), set(), set(), dict()
 
@@ -68,6 +79,8 @@ def load_polldevs(polldevs_conf: str) -> Tuple[Set, Set, Set, dict[str, str]]:
     for device in deleted_devices:
         del state.polldevs[device]
 
+    state.pollfile_mtime = modified_time
+
     return new_devices, deleted_devices, changed_devices, defaults
 
 

src/zino/state.py

@@ -25,6 +25,9 @@
 
 config: Configuration = Configuration()
 
+# Last time the pollfile was modified
+pollfile_mtime: Optional[float] = None
+
 
 class ZinoState(BaseModel):
     """Holds all state that Zino needs to persist between runtimes"""

tests/scheduler_test.py

@@ -1,4 +1,5 @@
 import logging
+from time import time
 from unittest.mock import Mock, patch
 
 import pytest
@@ -10,6 +11,7 @@
 
 class TestLoadPolldevs:
     @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
     @patch("zino.state.state", ZinoState())
     def test_should_return_all_new_devices_on_first_run(self, polldevs_conf):
         new_devices, deleted_devices, changed_devices, _ = scheduler.load_polldevs(polldevs_conf)
@@ -18,37 +20,46 @@ def test_should_return_all_new_devices_on_first_run(self, polldevs_conf):
         assert not changed_devices
 
     @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
     @patch("zino.state.state", ZinoState())
     def test_should_return_defaults_on_first_run(self, polldevs_conf):
         _, _, _, defaults = scheduler.load_polldevs(polldevs_conf)
         assert len(defaults) > 0
         assert "interval" in defaults
 
     @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
     @patch("zino.state.state", ZinoState())
     def test_should_return_deleted_devices_on_second_run(self, polldevs_conf, polldevs_conf_with_single_router):
         scheduler.load_polldevs(polldevs_conf)
-        new_devices, deleted_devices, changed_devices, _ = scheduler.load_polldevs(polldevs_conf_with_single_router)
+
+        # This needs to be patched since the mtime of the two conf fixtures is the same
+        with patch("zino.state.pollfile_mtime", last_run_time=time() - 60):
+            new_devices, deleted_devices, changed_devices, _ = scheduler.load_polldevs(polldevs_conf_with_single_router)
+
         assert not new_devices
         assert len(deleted_devices) > 0
         assert not changed_devices
 
     @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
     @patch("zino.state.state", ZinoState())
-    def test_should_return_no_new_or_deleted_devices_on_invalid_configuration(self, invalid_polldevs_conf):
+    def test__or_deleted_devices_on_invalid_configuration(self, invalid_polldevs_conf):
         new_devices, deleted_devices, changed_devices, _ = scheduler.load_polldevs(invalid_polldevs_conf)
         assert not new_devices
         assert not deleted_devices
         assert not changed_devices
 
     @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
     @patch("zino.state.state", ZinoState())
     def test_should_log_error_on_invalid_configuration(self, caplog, invalid_polldevs_conf):
         with caplog.at_level(logging.ERROR):
             scheduler.load_polldevs(invalid_polldevs_conf)
         assert "'lalala' is not a valid configuration line" in caplog.text
 
     @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
     @patch("zino.state.state", ZinoState())
     def test_should_return_changed_defaults(self, polldevs_conf, tmp_path):
         polldevs_with_changed_defaults = tmp_path.joinpath("changed-defaults-polldevs.cf")
@@ -69,10 +80,14 @@ def test_should_return_changed_defaults(self, polldevs_conf, tmp_path):
             )
 
         _, _, _, defaults = scheduler.load_polldevs(polldevs_conf)
-        _, _, _, changed_defaults = scheduler.load_polldevs(polldevs_with_changed_defaults)
+
+        # This needs to be patched since the mtime of the two conf fixtures is the same
+        with patch("zino.state.pollfile_mtime", last_run_time=time() - 60):
+            _, _, _, changed_defaults = scheduler.load_polldevs(polldevs_with_changed_defaults)
         assert defaults != changed_defaults
 
     @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
     @patch("zino.state.state", ZinoState())
     def test_should_return_changed_devices_on_changed_defaults(self, polldevs_conf, tmp_path):
         polldevs_with_changed_defaults = tmp_path.joinpath("changed-defaults-polldevs.cf")
@@ -93,12 +108,17 @@ def test_should_return_changed_devices_on_changed_defaults(self, polldevs_conf,
             )
 
         scheduler.load_polldevs(polldevs_conf)
-        new_devices, deleted_devices, changed_devices, _ = scheduler.load_polldevs(polldevs_with_changed_defaults)
+
+        # This needs to be patched since the mtime of the two conf fixtures is the same
+        with patch("zino.state.pollfile_mtime", last_run_time=time() - 60):
+            new_devices, deleted_devices, changed_devices, _ = scheduler.load_polldevs(polldevs_with_changed_defaults)
+
         assert not new_devices
         assert not deleted_devices
         assert changed_devices
 
     @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
     @patch("zino.state.state", ZinoState())
     def test_should_return_changed_devices_on_changed_interval(self, polldevs_conf, tmp_path):
         polldevs_with_changed_defaults = tmp_path.joinpath("changed-interval-polldevs.cf")
@@ -120,14 +140,44 @@ def test_should_return_changed_devices_on_changed_interval(self, polldevs_conf,
             )
 
         scheduler.load_polldevs(polldevs_conf)
-        new_devices, deleted_devices, changed_devices, _ = scheduler.load_polldevs(polldevs_with_changed_defaults)
+
+        # This needs to be patched since the mtime of the two conf fixtures is the same
+        with patch("zino.state.pollfile_mtime", last_run_time=time() - 60):
+            new_devices, deleted_devices, changed_devices, _ = scheduler.load_polldevs(polldevs_with_changed_defaults)
+
         assert not new_devices
         assert not deleted_devices
         assert changed_devices
 
+    @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
+    @patch("zino.state.state", ZinoState())
+    def test_should_return_no_new_or_deleted_devices_on_unchanged_configuration(self, polldevs_conf):
+        scheduler.load_polldevs(polldevs_conf)
+        new_devices, deleted_devices, _, _ = scheduler.load_polldevs(polldevs_conf)
+        assert not new_devices
+        assert not deleted_devices
+
+    @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
+    @patch("zino.state.state", ZinoState())
+    def test_should_return_no_new_or_deleted_devices_on_non_existent_pollfile(self, tmp_path):
+        new_devices, deleted_devices, _, _ = scheduler.load_polldevs(tmp_path / "non-existent-polldev.cf")
+        assert not new_devices
+        assert not deleted_devices
+
+    @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
+    @patch("zino.state.state", ZinoState())
+    def test_should_log_error_on_non_existent_pollfile(self, caplog, tmp_path):
+        with caplog.at_level(logging.ERROR):
+            scheduler.load_polldevs(tmp_path / "non-existent-polldev.cf")
+        assert "No such file or directory" in caplog.text
+
 
 class TestScheduleNewDevices:
     @patch("zino.state.polldevs", dict())
+    @patch("zino.state.pollfile_mtime", None)
     @patch("zino.state.state", ZinoState())
     def test_should_schedule_jobs_for_new_devices(self, polldevs_conf, mocked_scheduler):
         new_devices, _, _, _ = scheduler.load_polldevs(polldevs_conf)

zino.toml.example

@@ -22,7 +22,7 @@ period = 5
 # default "polldevs.cf"
 file = "polldevs.cf"
 
-# How often the pollfile is read, in minutes
+# How often the pollfile is checked for changes, in minutes
 # default 1 min
 period = 1