version 0.1.18: fixed space keeper runner failing when deleting file

nightskycam/space_keeper/runner.py

@@ -12,9 +12,17 @@
 from nightskyrunner.status import Level
 from nightskyrunner.wait_interrupts import RunnerWaitInterruptors
 
-from .utils import (DiskSpaceInfo, bits_to_human, bytes_to_human, convert_mb_to_bits,
-                    disk_space_info, disk_space_info_str, files_to_delete,
-                    folder_content, to_GB)
+from .utils import (
+    DiskSpaceInfo,
+    bits_to_human,
+    bytes_to_human,
+    convert_mb_to_bits,
+    disk_space_info,
+    disk_space_info_str,
+    files_to_delete,
+    folder_content,
+    to_GB,
+)
 
 
 @status_error
@@ -57,11 +65,11 @@ def __init__(
         super().__init__(name, config_getter, interrupts, core_frequency)
         self._nb_deleted: int = 0
 
-    def iterate(self):
+    def iterate(self) -> None:
 
         # reading this runner toml config file
         config = self.get_config()
-        folder = pathlib.Path(config["folder"])
+        folder = pathlib.Path(config["folder"])  # type: ignore
         threshold_MB = int(config["threshold_MB"])  # type: ignore
 
         # invalid configuration, exit with error
@@ -88,7 +96,9 @@ def iterate(self):
 
         # if disk is too full, to_delete will list the files to
         # delete
-        to_delete = files_to_delete(Path(folder), convert_mb_to_bits(threshold_MB))
+        to_delete = files_to_delete(
+            Path(folder), convert_mb_to_bits(threshold_MB)
+        )
 
         self._status.entries(
             SpaceKeeperRunnerEntries(
@@ -106,4 +116,3 @@ def iterate(self):
             self._status.activity("deleting files")
             list(map(os.remove, to_delete))
             self._nb_deleted += len(to_delete)
-            self._status.value("number of deleted files", self._nb_deleted)

nightskycam/utils/test_utils.py

@@ -2,24 +2,41 @@
 Functions useful for unit-testing.
 """
 
+from nightskyrunner.status import ErrorDict
 import logging
 import tempfile
 import time
 from contextlib import contextmanager
 from pathlib import Path, PosixPath
-from typing import (Any, Callable, Generator, Iterable, Optional, Tuple, Type,
-                    TypeVar, Union, cast)
+from typing import (
+    Any,
+    Callable,
+    Generator,
+    Iterable,
+    Optional,
+    Tuple,
+    Type,
+    TypeVar,
+    Union,
+    cast,
+)
 
 import tomli_w
 from nightskyrunner.config import Config
-from nightskyrunner.config_toml import (DynamicTomlConfigGetter,
-                                        DynamicTomlManagerConfigGetter,
-                                        TomlRunnerFactory)
+from nightskyrunner.config_toml import (
+    DynamicTomlConfigGetter,
+    DynamicTomlManagerConfigGetter,
+    TomlRunnerFactory,
+)
 from nightskyrunner.factories import BasicRunnerFactory, RunnerFactory
 from nightskyrunner.manager import FixedRunners, Manager
 from nightskyrunner.runner import Runner
-from nightskyrunner.status import (NoSuchStatusError, State, Status,
-                                   wait_for_status)
+from nightskyrunner.status import (
+    NoSuchStatusError,
+    State,
+    Status,
+    wait_for_status,
+)
 
 from nightskycam.utils.websocket_manager import websocket_server
 
@@ -194,7 +211,9 @@ def _get_runner_factory(
 
     # constructing the runner factory, i.e. the factories the manager will use
     # to instantiate the runners
-    runner_factories = [_get_runner_factory(rcc) for rcc in runner_class_configs]
+    runner_factories = [
+        _get_runner_factory(rcc) for rcc in runner_class_configs
+    ]
 
     # the manager config getter, i.e. the class the manager will use to configure
     # itself, i.e. selecting which runner to instantiate and start.
@@ -226,6 +245,24 @@ def get_runner_error(runner_name: str) -> Optional[str]:
         return None
 
 
+def had_error(runner_name: str) -> bool:
+    """
+    Returns True if the runner ever encountered an error in the past,
+    i.e. the field error or previous error is not None.
+    """
+    status = Status.retrieve(runner_name)
+    d = status.get()
+    try:
+        error: ErrorDict = d["error"]
+    except KeyError:
+        return False
+    if "message" in error and error["message"] is not None:
+        return True
+    if "previous" in error and error["previous"] is not None:
+        return True
+    return False
+
+
 def exception_on_error_state(runner_names: Union[str, Iterable[str]]) -> None:
     """
     Check the state associated with each runner name.
@@ -241,6 +278,9 @@ def exception_on_error_state(runner_names: Union[str, Iterable[str]]) -> None:
     If there is no status for one of the runner (i.e. no
     runner of that name has been started), nothing occurs
     (no exception is raised).
+
+    Note: it is likely better to use 'had_error', which also check
+    for "previous" error state.
     """
     if type(runner_names) == str:
         runner_names = (runner_names,)
@@ -313,7 +353,9 @@ def wait_for(
         time.sleep(time_sleep)
 
 
-def websocket_connection_test(runner_class: Type[Runner], port, config: Config) -> None:
+def websocket_connection_test(
+    runner_class: Type[Runner], port, config: Config
+) -> None:
     """
     It is assumed 'runner_class' is a runner requiring an active websocket connection.
     This function will test that the runner is in a 'running' state when a
@@ -399,7 +441,9 @@ class ConfigTester:
         should be in 'supported_values' (otherwise a KeyError is raised).
     """
 
-    def __init__(self, supported_values: Config, not_supported_values: Config) -> None:
+    def __init__(
+        self, supported_values: Config, not_supported_values: Config
+    ) -> None:
         self._supported = supported_values
         self._not_supported = not_supported_values
         for key in not_supported_values:
@@ -415,7 +459,9 @@ def keys(self) -> set[str]:
         """
         return set(self._not_supported.keys())
 
-    def get_config(self, unsupported: Union[str, Iterable[str]] = tuple()) -> Config:
+    def get_config(
+        self, unsupported: Union[str, Iterable[str]] = tuple()
+    ) -> Config:
         """
         If unsupported is empty, returns the 'supported_values' configuration.
         If unsupported is not empty, returns the 'supported_values' configuration
@@ -458,7 +504,9 @@ def _get_status(runner: Union[str, Type[Runner]]) -> str:
         runner_ = runner.__name__
     else:
         runner_ = str(runner)
-    return ",".join([f"{k}: {v}" for k, v in Status.retrieve(runner_).get().items()])
+    return ",".join(
+        [f"{k}: {v}" for k, v in Status.retrieve(runner_).get().items()]
+    )
 
 
 def configuration_test(
@@ -482,7 +530,9 @@ def configuration_test(
         with get_manager((runner_class, config_file)):
             # waiting for the runner to start and go into a "running" state
             # (no error because the config is correct)
-            if not wait_for_status(runner_name, State.running, timeout=timeout):
+            if not wait_for_status(
+                runner_name, State.running, timeout=timeout
+            ):
                 raise RuntimeError(
                     f"{runner_class.__name__} did not switch to running state "
                     "when starting with a suitable configuration. "
@@ -494,7 +544,9 @@ def configuration_test(
                 config_tester.set_config(config_file, unsupported=config_key)
 
                 # the runner should switch to an error state
-                if not wait_for_status(runner_name, State.error, timeout=timeout):
+                if not wait_for_status(
+                    runner_name, State.error, timeout=timeout
+                ):
                     raise RuntimeError(
                         f"{runner_class.__name__} did not switch to error state "
                         f"upon unsupported configuration value for key {config_key}. "
@@ -505,7 +557,9 @@ def configuration_test(
                 config_tester.set_config(config_file, unsupported=tuple())
 
                 # runner should return to a "running" state.
-                if not wait_for_status(runner_name, State.running, timeout=timeout):
+                if not wait_for_status(
+                    runner_name, State.running, timeout=timeout
+                ):
                     raise RuntimeError(
                         f"{runner_class.__name__} did not switch to running state "
                         "when switching back to a suitable configuration. "

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "nightskycam"
-version = "0.1.17"
+version = "0.1.18"
 description = "taking pictures at night"
 authors = ["Vincent Berenz <[email protected]>"]
 packages = [{ include = "nightskycam" }]

tests/test_cam_runner.py

@@ -14,10 +14,14 @@
 from nightskycam.cams.runner import CamRunner
 from nightskycam.dummycams.runner import DummyCamRunner
 from nightskycam.location_info.runner import LocationInfoRunner
-from nightskycam.utils.test_utils import (ConfigTester, configuration_test,
-                                          exception_on_error_state,
-                                          get_manager, runner_started,
-                                          wait_for)
+from nightskycam.utils.test_utils import (
+    ConfigTester,
+    configuration_test,
+    had_error,
+    get_manager,
+    runner_started,
+    wait_for,
+)
 from nightskyrunner.config import Config
 from nightskyrunner.shared_memory import SharedMemory
 from nightskyrunner.status import State, wait_for_status
@@ -231,7 +235,9 @@ def test_get_local_info(reset_memory) -> None:
     assert get_weather is None
     assert get_cloud_cover is None
 
-    def _wait_for_local_info(night: bool, weather: str, cloud_cover: int) -> bool:
+    def _wait_for_local_info(
+        night: bool, weather: str, cloud_cover: int
+    ) -> bool:
         get_night, get_weather, get_cloud_cover = utils.get_local_info()
         return all(
             [
@@ -252,15 +258,19 @@ def _wait_for_local_info(night: bool, weather: str, cloud_cover: int) -> bool:
     wait_for(_wait_for_local_info, True, args=(night, weather, cloud_cover))
 
     time.sleep(0.2)
-    get_night, get_weather, get_cloud_cover = utils.get_local_info(deprecation=0.15)
+    get_night, get_weather, get_cloud_cover = utils.get_local_info(
+        deprecation=0.15
+    )
     assert get_night is None
     assert get_weather is None
     assert get_cloud_cover is None
 
 
 class _RunnerConfig:
     @classmethod
-    def get_config(cls, destination_folder: Path, unsupported: bool = False) -> Config:
+    def get_config(
+        cls, destination_folder: Path, unsupported: bool = False
+    ) -> Config:
         if unsupported:
             return {
                 "frequency": 0.0,
@@ -349,9 +359,9 @@ def _image_generated(destination_folder: Path) -> bool:
         # pictures in tmp_dir
         wait_for(runner_started, True, args=(DummyCamRunner.__name__,))
         wait_for_status(DummyCamRunner.__name__, State.running, timeout=2.0)
-        exception_on_error_state(DummyCamRunner.__name__)
+        assert not had_error(DummyCamRunner.__name__)
         wait_for(_image_generated, True, args=(tmp_dir,))
-        exception_on_error_state(DummyCamRunner.__name__)
+        assert not had_error(DummyCamRunner.__name__)
 
 
 def test_wait_duration() -> None:
@@ -394,4 +404,4 @@ def test_no_local_info(tmp_dir) -> None:
     with get_manager((DummyCamRunner, config)):
         wait_for(runner_started, True, args=(DummyCamRunner.__name__,))
         wait_for_status(DummyCamRunner.__name__, State.running, timeout=2.0)
-        exception_on_error_state(DummyCamRunner.__name__)
+        assert not had_error(DummyCamRunner.__name__)

tests/test_ftp_runner.py

@@ -9,10 +9,14 @@
 from nightskycam.ftp.runner import FtpRunner, _UploadSpeed
 from nightskycam.utils.filename import get_filename
 from nightskycam.utils.ftp import FtpConfig, FtpServer, get_ftp
-from nightskycam.utils.test_utils import (ConfigTester, configuration_test,
-                                          exception_on_error_state,
-                                          get_manager, runner_started,
-                                          wait_for)
+from nightskycam.utils.test_utils import (
+    ConfigTester,
+    configuration_test,
+    had_error,
+    get_manager,
+    runner_started,
+    wait_for,
+)
 from nightskyrunner.config import Config
 from nightskyrunner.shared_memory import SharedMemory
 
@@ -140,7 +144,9 @@ def test_ftp(ftp_server) -> None:
     config: FtpConfig = ftp_server
 
     if not config.folder:
-        raise ValueError("the configuration folder needs to be set for this test")
+        raise ValueError(
+            "the configuration folder needs to be set for this test"
+        )
 
     # the files will be copied in this subfolders
     # of the ftp server
@@ -271,8 +277,12 @@ def get_config_tester(
         cls, system_name: str, remote_subdir: str, folder: Path
     ) -> ConfigTester:
         return ConfigTester(
-            cls.get_config(system_name, remote_subdir, folder, unsupported=False),
-            cls.get_config(system_name, remote_subdir, folder, unsupported=True),
+            cls.get_config(
+                system_name, remote_subdir, folder, unsupported=False
+            ),
+            cls.get_config(
+                system_name, remote_subdir, folder, unsupported=True
+            ),
         )
 
 
@@ -319,13 +329,17 @@ def test_ftp_runner(tmp_dir, ftp_server, reset_memory) -> None:
     Testing instances of FtpRunner behave as expected
     """
 
-    def _nb_files_decreased(tmp_dir: Path, nb_files: int, nb_uploaded: int) -> bool:
+    def _nb_files_decreased(
+        tmp_dir: Path, nb_files: int, nb_uploaded: int
+    ) -> bool:
         return len(_list_files(tmp_dir)) <= nb_files - nb_uploaded
 
     ftp_config: FtpConfig = ftp_server
     system_name = "test_system"
     remote_subdir = "test"
-    config: Config = _FtpRunnerConfig.get_config(system_name, remote_subdir, tmp_dir)
+    config: Config = _FtpRunnerConfig.get_config(
+        system_name, remote_subdir, tmp_dir
+    )
     nb_files = 50
     nb_uploaded = 6
     day = "2023_01_01"
@@ -335,16 +349,18 @@ def _nb_files_decreased(tmp_dir: Path, nb_files: int, nb_uploaded: int) -> bool:
     with get_manager((FtpRunner, config)):
         # checking runner does not raise exception upon no files to upload
         wait_for(runner_started, True, args=(FtpRunner.__name__,))
-        exception_on_error_state(FtpRunner.__name__)
+        assert not had_error(FtpRunner.__name__)
 
         # adding files to upload
         files_to_upload = _write_ordered_date_formated_files(
             tmp_dir, start_date, nb_files
         )
 
         # waiting for at least nb_uploaded files to be uploaded
-        wait_for(_nb_files_decreased, True, args=(tmp_dir, nb_files, nb_uploaded))
-        exception_on_error_state(FtpRunner.__name__)
+        wait_for(
+            _nb_files_decreased, True, args=(tmp_dir, nb_files, nb_uploaded)
+        )
+        assert not had_error(FtpRunner.__name__)
 
     # checking files have been uploaded
     target_dir = ftp_config.folder / remote_subdir / system_name / day  # type: ignore