core:libs:commonwealth:settings: Make save atomic

core/libs/commonwealth/src/commonwealth/settings/bases/pydantic_base.py

@@ -1,6 +1,7 @@
 import abc
 import json
 import pathlib
+import os
 from typing import Any, ClassVar, Dict, List
 
 from loguru import logger
@@ -106,11 +107,20 @@ def save(self, file_path: pathlib.Path) -> None:
             # We call this to allow users to initialize the settings from another source if needed
             self.on_settings_created(file_path)
 
-        with open(file_path, "w", encoding="utf-8") as settings_file:
-            logger.debug(f"Saving settings on: {file_path}")
+        # Prepare data prior to operation
+        logger.debug(f"Saving settings on: {file_path}")
+        json_data = self.dict()
 
-            json_data = self.dict()
+        # Create a temporary file in same directory, write and rename it to the original file
+        temp_file = file_path.with_suffix(".tmp")
+        with open(temp_file, "w", encoding="utf-8") as settings_file:
             json.dump(json_data, settings_file, indent=4)
+            # Ensure data is written to disk
+            settings_file.flush()
+            os.fsync(settings_file.fileno())
+        # Replace the original file with the temporary file, this operation is atomic if in the same filesystem
+        # https://docs.python.org/3/library/os.html#os.replace
+        temp_file.replace(file_path)
 
     def reset(self) -> None:
         """Reset internal data to default values"""

core/libs/commonwealth/src/commonwealth/settings/bases/pykson_base.py

@@ -1,6 +1,7 @@
 import abc
 import json
 import pathlib
+import os
 from typing import Any, Dict
 
 import pykson  # type: ignore
@@ -90,9 +91,20 @@ def save(self, file_path: pathlib.Path) -> None:
         parent_path = file_path.parent.absolute()
         parent_path.mkdir(parents=True, exist_ok=True)
 
-        with open(file_path, "w", encoding="utf-8") as settings_file:
-            logger.debug(f"Saving settings on: {file_path}")
-            settings_file.write(json.dumps(json.loads(Pykson().to_json(self)), indent=4))
+        # Prepare data prior to operation
+        logger.debug(f"Saving settings on: {file_path}")
+        json_data = json.dumps(json.loads(Pykson().to_json(self)), indent=4)
+
+        # Create a temporary file in same directory, write and rename it to the original file
+        temp_file = file_path.with_suffix(".tmp")
+        with open(temp_file, "w", encoding="utf-8") as settings_file:
+            settings_file.write(json_data)
+            # Ensure data is written to disk
+            settings_file.flush()
+            os.fsync(settings_file.fileno())
+        # Replace the original file with the temporary file, this operation is atomic if in the same filesystem
+        # https://docs.python.org/3/library/os.html#os.replace
+        temp_file.replace(file_path)
 
     def reset(self) -> None:
         """Reset internal data to default values"""

core/libs/commonwealth/src/commonwealth/settings/managers/pydantic_manager.py

@@ -103,6 +103,10 @@ def save(self) -> None:
     def load(self) -> None:
         """Load settings"""
 
+        # TODO: We could try to restore the settings from the temporary file if the main is not found or valid
+        # Clear temporary files that could be left from a previous operation
+        self._clear_temp_files()
+
         def get_settings_version_from_filename(filename: pathlib.Path) -> int:
             result = re.search(f"{PydanticManager.SETTINGS_NAME_PREFIX}(\\d+)", filename.name)
             assert result
@@ -128,3 +132,11 @@ def get_settings_version_from_filename(filename: pathlib.Path) -> int:
                 logger.debug("Invalid settings, going to try another file:", exception)
 
         self._settings = PydanticManager.load_from_file(self.settings_type, self.settings_file_path())
+
+    def _clear_temp_files(self) -> None:
+        """Clear temporary files"""
+        for temp_file in self.config_folder.glob("*.tmp"):
+            try:
+                temp_file.unlink()
+            except Exception as exception:
+                logger.debug(f"Failed to clear temporary file {temp_file}: {exception}")

core/libs/commonwealth/src/commonwealth/settings/managers/pykson_manager.py

@@ -99,6 +99,10 @@ def save(self) -> None:
     def load(self) -> None:
         """Load settings"""
 
+        # TODO: We could try to restore the settings from the temporary file if the main is not found or valid
+        # Clear temporary files that could be left from a previous operation
+        self._clear_temp_files()
+
         def get_settings_version_from_filename(filename: pathlib.Path) -> int:
             result = re.search(f"{PyksonManager.SETTINGS_NAME_PREFIX}(\\d+)", filename.name)
             assert result
@@ -124,3 +128,11 @@ def get_settings_version_from_filename(filename: pathlib.Path) -> int:
                 logger.debug("Invalid settings, going to try another file:", exception)
 
         self._settings = PyksonManager.load_from_file(self.settings_type, self.settings_file_path())
+
+    def _clear_temp_files(self) -> None:
+        """Clear temporary files"""
+        for temp_file in self.config_folder.glob("*.tmp"):
+            try:
+                temp_file.unlink()
+            except Exception as exception:
+                logger.debug(f"Failed to clear temporary file {temp_file}: {exception}")