Improve the backup mechanism of the configuration file

aiida/backends/tests/utils/configuration.py

@@ -54,6 +54,7 @@ def temporary_config_instance():
     current_profile_name = None
     temporary_config_directory = None
 
+    from aiida.common.utils import Capturing
     from aiida.manage import configuration
     from aiida.manage.configuration import settings, load_profile, reset_profile
 
@@ -75,7 +76,11 @@ def temporary_config_instance():
 
         # Create the instance base directory structure, the config file and a dummy profile
         create_instance_directories()
-        configuration.CONFIG = configuration.load_config(create=True)
+
+        # The constructor of `Config` called by `load_config` will print warning messages about migrating it
+        with Capturing():
+            configuration.CONFIG = configuration.load_config(create=True)
+
         profile = create_mock_profile(name=profile_name, repository_dirpath=temporary_config_directory)
 
         # Add the created profile and set it as the default

aiida/manage/configuration/__init__.py

@@ -80,7 +80,6 @@ def load_config(create=False):
     :rtype: :class:`~aiida.manage.configuration.config.Config`
     :raises aiida.common.MissingConfigurationError: if the configuration file could not be found and create=False
     """
-    import io
     import os
     from aiida.common import exceptions
     from .config import Config
@@ -116,17 +115,10 @@ def load_config(create=False):
     if not os.path.isfile(filepath) and not create:
         raise exceptions.MissingConfigurationError('configuration file {} does not exist'.format(filepath))
 
-    # If it doesn't exist, just create the file
-    if not os.path.isfile(filepath):
-        from aiida.common import json
-        with io.open(filepath, 'ab') as handle:
-            json.dump({}, handle)
-
     try:
         config = Config.from_file(filepath)
     except ValueError:
         raise exceptions.ConfigurationError('configuration file {} contains invalid JSON'.format(filepath))
-    config.store()
 
     return config
 

aiida/manage/configuration/config.py

@@ -20,7 +20,6 @@
 
 from .options import get_option, parse_option, NO_DEFAULT
 from .profile import Profile
-from .settings import DEFAULT_UMASK, DEFAULT_CONFIG_INDENT_SIZE
 
 __all__ = ('Config',)
 
@@ -39,24 +38,30 @@ class Config(object):  # pylint: disable=too-many-public-methods
     def from_file(cls, filepath):
         """Instantiate a configuration object from the contents of a given file.
 
+        .. note:: if the filepath does not exist an empty file will be created with the default configuration.
+
         :param filepath: the absolute path to the configuration file
         :return: `Config` instance
         """
         from aiida.cmdline.utils import echo
         from .migrations import check_and_migrate_config, config_needs_migrating
 
-        with io.open(filepath, 'r', encoding='utf8') as handle:
-            config = json.load(handle)
-
-        # If the configuration file needs to be migrated, first create a specific backup so it can easily be reverted
-        if config_needs_migrating(config):
-            echo.echo_warning('current configuration file `{}` is outdated and will be migrated'.format(filepath))
-            filepath_backup = cls._backup(filepath)
-            echo.echo_warning('original backed up to `{}`'.format(filepath_backup))
+        try:
+            with io.open(filepath, 'r', encoding='utf8') as handle:
+                config = json.load(handle)
+        except (IOError, OSError):
+            config = Config(filepath, check_and_migrate_config({}))
+            config.store()
+        else:
+            # If the configuration file needs to be migrated first create a specific backup so it can easily be reverted
+            if config_needs_migrating(config):
+                echo.echo_warning('current configuration file `{}` is outdated and will be migrated'.format(filepath))
+                filepath_backup = cls._backup(filepath)
+                echo.echo_warning('original backed up to `{}`'.format(filepath_backup))
 
-        config = check_and_migrate_config(config)
+            config = Config(filepath, check_and_migrate_config(config))
 
-        return Config(filepath, config)
+        return config
 
     @classmethod
     def _backup(cls, filepath):
@@ -73,11 +78,7 @@ def _backup(cls, filepath):
         while not filepath_backup or os.path.isfile(filepath_backup):
             filepath_backup = '{}.{}'.format(filepath, timezone.now().strftime('%Y%m%d-%H%M%S.%f'))
 
-        try:
-            umask = os.umask(DEFAULT_UMASK)
-            shutil.copy(filepath, filepath_backup)
-        finally:
-            os.umask(umask)
+        shutil.copy(filepath, filepath_backup)
 
         return filepath_backup
 
@@ -366,16 +367,45 @@ def get_option(self, option_name, scope=None, default=True):
         return value
 
     def store(self):
-        """Write the current config to file."""
-        if os.path.isfile(self.filepath):
-            self._backup(self.filepath)
+        """Write the current config to file.
+
+        .. note:: if the configuration file already exists on disk and its contents differ from those in memory, a
+            backup of the original file on disk will be created before overwriting it.
+
+        :return: self
+        """
+        import tempfile
+        from aiida.common.files import md5_from_filelike, md5_file
+
+        # If the filepath of this configuration does not yet exist, simply write it.
+        if not os.path.isfile(self.filepath):
+            with io.open(self.filepath, 'wb') as handle:
+                self._write(handle)
+            return self
+
+        # Otherwise, we write the content to a temporary file and compare its md5 checksum with the current config on
+        # disk. When the checksums differ, we first create a backup and only then overwrite the existing file.
+        with tempfile.NamedTemporaryFile() as handle:
+            self._write(handle)
+            handle.seek(0)
+
+            if md5_from_filelike(handle) != md5_file(self.filepath):
+                self._backup(self.filepath)
+
+            shutil.copy(handle.name, self.filepath)
+
+        return self
+
+    def _write(self, filelike):
+        """Write the contents of `self.dictionary` to the given file handle.
+
+        :param filelike: the filelike object to write the current configuration to
+        """
+        from .settings import DEFAULT_UMASK, DEFAULT_CONFIG_INDENT_SIZE
 
         umask = os.umask(DEFAULT_UMASK)
 
         try:
-            with io.open(self.filepath, 'wb') as handle:
-                json.dump(self.dictionary, handle, indent=DEFAULT_CONFIG_INDENT_SIZE)
+            json.dump(self.dictionary, filelike, indent=DEFAULT_CONFIG_INDENT_SIZE)
         finally:
             os.umask(umask)
-
-        return self