From 69f6e62877dcaafed3dec109767cbcd52c1d1ca7 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 14 Aug 2023 11:50:59 +0000 Subject: [PATCH 1/5] test(backups): more checks regarding tmpdirs and mounting --- .../backup/backuppers/restic_backupper.py | 13 +++++++++---- tests/test_graphql/test_backup.py | 9 +++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 3a5fc49..418aa5b 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -126,7 +126,9 @@ class ResticBackupper(AbstractBackupper): output, ) - def mount_repo(self, mount_directory): + def mount_repo(self, mount_directory: str) -> subprocess.Popen: + if not exists(mount_directory): + raise FileNotFoundError("no such directory to mount at: ", mount_directory) mount_command = self.restic_command("mount", mount_directory) mount_command.insert(0, "nohup") handle = subprocess.Popen( @@ -139,7 +141,7 @@ class ResticBackupper(AbstractBackupper): raise IOError("failed to mount dir ", mount_directory) return handle - def unmount_repo(self, mount_directory): + def unmount_repo(self, mount_directory: str) -> None: mount_command = ["umount", "-l", mount_directory] with subprocess.Popen( mount_command, stdout=subprocess.PIPE, shell=False @@ -147,10 +149,10 @@ class ResticBackupper(AbstractBackupper): output = handle.communicate()[0].decode("utf-8") # TODO: check for exit code? if "error" in output.lower(): - return IOError("failed to unmount dir ", mount_directory, ": ", output) + raise IOError("failed to unmount dir ", mount_directory, ": ", output) if not listdir(mount_directory) == []: - return IOError("failed to unmount dir ", mount_directory) + raise IOError("failed to unmount dir ", mount_directory) @staticmethod def __flatten_list(list_to_flatten): @@ -363,6 +365,9 @@ class ResticBackupper(AbstractBackupper): self._raw_verified_restore(snapshot_id, target=temp_dir) snapshot_root = temp_dir else: # attempting inplace restore via mount + sync + assert exists( + temp_dir + ) # paranoid check, TemporaryDirectory is supposedly created self.mount_repo(temp_dir) snapshot_root = join(temp_dir, "ids", snapshot_id) diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 1990ef7..9736f91 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -8,6 +8,8 @@ from os import urandom from datetime import datetime, timedelta, timezone from subprocess import Popen +import tempfile + import selfprivacy_api.services as services from selfprivacy_api.services import Service, get_all_services @@ -806,3 +808,10 @@ def test_operations_while_locked(backups, dummy_service): # check that no locks were left Backups.provider().backupper.lock() Backups.provider().backupper.unlock() + + +# a paranoid check to weed out problems with tempdirs that are not dependent on us +def test_tempfile(): + with tempfile.TemporaryDirectory() as temp: + assert path.exists(temp) + assert not path.exists(temp) From c89f9cf89d06aeee6997e80b0376dd50a86e373b Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 14 Aug 2023 12:43:44 +0000 Subject: [PATCH 2/5] feature(backups): do not rely on mounting --- .../backup/backuppers/restic_backupper.py | 33 +++++++++---------- tests/test_graphql/test_backup.py | 11 +++++++ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index 418aa5b..a935520 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -9,8 +9,9 @@ from typing import List, TypeVar, Callable from collections.abc import Iterable from json.decoder import JSONDecodeError from os.path import exists, join -from os import listdir +from os import listdir, mkdir from time import sleep +from shutil import rmtree from selfprivacy_api.backup.util import output_yielder, sync from selfprivacy_api.backup.backuppers import AbstractBackupper @@ -364,23 +365,21 @@ class ResticBackupper(AbstractBackupper): if verify: self._raw_verified_restore(snapshot_id, target=temp_dir) snapshot_root = temp_dir - else: # attempting inplace restore via mount + sync - assert exists( - temp_dir - ) # paranoid check, TemporaryDirectory is supposedly created - self.mount_repo(temp_dir) - snapshot_root = join(temp_dir, "ids", snapshot_id) + for folder in folders: + src = join(snapshot_root, folder.strip("/")) + if not exists(src): + raise ValueError( + f"No such path: {src}. We tried to find {folder}" + ) + dst = folder + sync(src, dst) - assert snapshot_root is not None - for folder in folders: - src = join(snapshot_root, folder.strip("/")) - if not exists(src): - raise ValueError(f"No such path: {src}. We tried to find {folder}") - dst = folder - sync(src, dst) - - if not verify: - self.unmount_repo(temp_dir) + else: # attempting inplace restore + for folder in folders: + rmtree(folder) + mkdir(folder) + self._raw_verified_restore(snapshot_id, target="/") + return def _raw_verified_restore(self, snapshot_id, target="/"): """barebones restic restore""" diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 9736f91..25eaaf4 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -746,6 +746,17 @@ def test_mount_umount(backups, dummy_service, tmpdir): assert len(listdir(mountpoint)) == 0 +def test_mount_nonexistent(backups, dummy_service, tmpdir): + backupper = Backups.provider().backupper + assert isinstance(backupper, ResticBackupper) + + mountpoint = tmpdir / "nonexistent" + assert not path.exists(mountpoint) + + with pytest.raises(FileNotFoundError): + handle = backupper.mount_repo(mountpoint) + + def test_move_blocks_backups(backups, dummy_service, restore_strategy): snap = Backups.back_up(dummy_service) job = Jobs.add( From d621ca64497a67b72b194a1d5002b0a764e56655 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 14 Aug 2023 12:50:45 +0000 Subject: [PATCH 3/5] refactor(backups): clean up unused mounting tools --- .../backup/backuppers/restic_backupper.py | 28 ----------------- tests/test_graphql/test_backup.py | 30 ------------------- 2 files changed, 58 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index a935520..e954b65 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -127,34 +127,6 @@ class ResticBackupper(AbstractBackupper): output, ) - def mount_repo(self, mount_directory: str) -> subprocess.Popen: - if not exists(mount_directory): - raise FileNotFoundError("no such directory to mount at: ", mount_directory) - mount_command = self.restic_command("mount", mount_directory) - mount_command.insert(0, "nohup") - handle = subprocess.Popen( - mount_command, - stdout=subprocess.DEVNULL, - shell=False, - ) - sleep(2) - if "ids" not in listdir(mount_directory): - raise IOError("failed to mount dir ", mount_directory) - return handle - - def unmount_repo(self, mount_directory: str) -> None: - mount_command = ["umount", "-l", mount_directory] - with subprocess.Popen( - mount_command, stdout=subprocess.PIPE, shell=False - ) as handle: - output = handle.communicate()[0].decode("utf-8") - # TODO: check for exit code? - if "error" in output.lower(): - raise IOError("failed to unmount dir ", mount_directory, ": ", output) - - if not listdir(mount_directory) == []: - raise IOError("failed to unmount dir ", mount_directory) - @staticmethod def __flatten_list(list_to_flatten): """string-aware list flattener""" diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py index 25eaaf4..6878de1 100644 --- a/tests/test_graphql/test_backup.py +++ b/tests/test_graphql/test_backup.py @@ -727,36 +727,6 @@ def test_sync_nonexistent_src(dummy_service): sync(src, dst) -# Restic lowlevel -def test_mount_umount(backups, dummy_service, tmpdir): - Backups.back_up(dummy_service) - backupper = Backups.provider().backupper - assert isinstance(backupper, ResticBackupper) - - mountpoint = tmpdir / "mount" - makedirs(mountpoint) - assert path.exists(mountpoint) - assert len(listdir(mountpoint)) == 0 - - handle = backupper.mount_repo(mountpoint) - assert len(listdir(mountpoint)) != 0 - - backupper.unmount_repo(mountpoint) - # handle.terminate() - assert len(listdir(mountpoint)) == 0 - - -def test_mount_nonexistent(backups, dummy_service, tmpdir): - backupper = Backups.provider().backupper - assert isinstance(backupper, ResticBackupper) - - mountpoint = tmpdir / "nonexistent" - assert not path.exists(mountpoint) - - with pytest.raises(FileNotFoundError): - handle = backupper.mount_repo(mountpoint) - - def test_move_blocks_backups(backups, dummy_service, restore_strategy): snap = Backups.back_up(dummy_service) job = Jobs.add( From d6cf2abdc23444e737bde58dd8b47f331c5f739b Mon Sep 17 00:00:00 2001 From: Inex Code Date: Wed, 23 Aug 2023 14:51:01 +0300 Subject: [PATCH 4/5] style: remove unused imports --- .../backup/backuppers/restic_backupper.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/selfprivacy_api/backup/backuppers/restic_backupper.py b/selfprivacy_api/backup/backuppers/restic_backupper.py index e954b65..f508368 100644 --- a/selfprivacy_api/backup/backuppers/restic_backupper.py +++ b/selfprivacy_api/backup/backuppers/restic_backupper.py @@ -9,8 +9,7 @@ from typing import List, TypeVar, Callable from collections.abc import Iterable from json.decoder import JSONDecodeError from os.path import exists, join -from os import listdir, mkdir -from time import sleep +from os import mkdir from shutil import rmtree from selfprivacy_api.backup.util import output_yielder, sync @@ -33,12 +32,12 @@ def unlocked_repo(func: T) -> T: def inner(self: ResticBackupper, *args, **kwargs): try: return func(self, *args, **kwargs) - except Exception as e: - if "unable to create lock" in str(e): + except Exception as error: + if "unable to create lock" in str(error): self.unlock() return func(self, *args, **kwargs) else: - raise e + raise error # Above, we manually guarantee that the type returned is compatible. return inner # type: ignore @@ -293,8 +292,8 @@ class ResticBackupper(AbstractBackupper): break if "unable" in line: raise ValueError(line) - except Exception as e: - raise ValueError("could not lock repository") from e + except Exception as error: + raise ValueError("could not lock repository") from error @unlocked_repo def restored_size(self, snapshot_id: str) -> int: From f2c972ed5f0790d9a3eb4271c3114bdbf9c3acd3 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Wed, 23 Aug 2023 14:51:15 +0300 Subject: [PATCH 5/5] chore: bump version --- selfprivacy_api/dependencies.py | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/selfprivacy_api/dependencies.py b/selfprivacy_api/dependencies.py index fb974e8..095d087 100644 --- a/selfprivacy_api/dependencies.py +++ b/selfprivacy_api/dependencies.py @@ -27,4 +27,4 @@ async def get_token_header( def get_api_version() -> str: """Get API version""" - return "2.3.0" + return "2.3.1" diff --git a/setup.py b/setup.py index 684f54f..99f0679 100755 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ from setuptools import setup, find_packages setup( name="selfprivacy_api", - version="2.3.0", + version="2.3.1", packages=find_packages(), scripts=[ "selfprivacy_api/app.py",