improved error/exception handling, fixes #6

src/borgstore/backends/_base.py

@@ -41,15 +41,6 @@ def validate_name(name):
 
 
 class BackendBase(ABC):
- class BackendException(Exception):
- """backend exception base class"""
-
- class MustNotBeOpen(BackendException):
- """backend must not be open"""
-
- class MustBeOpen(BackendException):
- """backend must be open"""
-
  @abstractmethod
  def create(self):
  """create (initialize) a backend storage"""

src/borgstore/backends/errors.py

@@ -0,0 +1,22 @@
+class BackendError(Exception):
+ """Base class for exceptions in this module."""
+
+
+class BackendAlreadyExists(BackendError):
+ """Raised when a backend already exists."""
+
+
+class BackendDoesNotExist(BackendError):
+ """Raised when a backend does not exist."""
+
+
+class BackendMustNotBeOpen(BackendError):
+ """Backend must not be open."""
+
+
+class BackendMustBeOpen(BackendError):
+ """Backend must be open."""
+
+
+class ObjectNotFound(BackendError):
+ """Object not found."""

src/borgstore/backends/posixfs.py

@@ -9,6 +9,7 @@
 import tempfile
 
 from ._base import BackendBase, ItemInfo, validate_name
+from .errors import BackendAlreadyExists, BackendDoesNotExist, BackendMustNotBeOpen, BackendMustBeOpen, ObjectNotFound
 from ..constants import TMP_SUFFIX
 
 
@@ -30,42 +31,56 @@ def __init__(self, path):
 
  def create(self):
  if self.opened:
- raise self.MustNotBeOpen()
- self.base_path.mkdir()
+ raise BackendMustNotBeOpen()
+ try:
+ self.base_path.mkdir()
+ except FileExistsError:
+ raise BackendAlreadyExists(f"posixfs storage base path already exists: {self.base_path}")
 
  def destroy(self):
  if self.opened:
- raise self.MustNotBeOpen()
- shutil.rmtree(os.fspath(self.base_path))
+ raise BackendMustNotBeOpen()
+ try:
+ shutil.rmtree(os.fspath(self.base_path))
+ except FileNotFoundError:
+ raise BackendDoesNotExist(f"posixfs storage base path does not exist: {self.base_path}")
 
  def open(self):
  if self.opened:
- raise self.MustNotBeOpen()
+ raise BackendMustNotBeOpen()
  if not self.base_path.is_dir():
- raise TypeError(f"storage base path does not exist or is not a directory: {self.base_path}")
+ raise BackendDoesNotExist(
+ f"posixfs storage base path does not exist or is not a directory: {self.base_path}"
+ )
  self.opened = True
 
  def close(self):
  if not self.opened:
- raise self.MustBeOpen()
+ raise BackendMustBeOpen()
  self.opened = False
 
  def _validate_join(self, name):
  validate_name(name)
  return self.base_path / name
 
  def mkdir(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  path = self._validate_join(name)
  path.mkdir(parents=True, exist_ok=True)
 
  def rmdir(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  path = self._validate_join(name)
  try:
  path.rmdir()
  except FileNotFoundError:
- raise KeyError(name) from None
+ raise ObjectNotFound(name) from None
 
  def info(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  path = self._validate_join(name)
  try:
  st = path.stat()
@@ -77,16 +92,20 @@ def info(self, name):
  return ItemInfo(name=path.name, exists=True, directory=is_dir, size=size)
 
  def load(self, name, *, size=None, offset=0):
+ if not self.opened:
+ raise BackendMustBeOpen()
  path = self._validate_join(name)
  try:
  with path.open("rb") as f:
  if offset > 0:
  f.seek(offset)
  return f.read(-1 if size is None else size)
  except FileNotFoundError:
- raise KeyError(name) from None
+ raise ObjectNotFound(name) from None
 
  def store(self, name, value):
+ if not self.opened:
+ raise BackendMustBeOpen()
  path = self._validate_join(name)
  tmp_dir = path.parent
  tmp_dir.mkdir(parents=True, exist_ok=True)
@@ -106,22 +125,28 @@ def store(self, name, value):
  raise
 
  def delete(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  path = self._validate_join(name)
  try:
  path.unlink()
  except FileNotFoundError:
- raise KeyError(name) from None
+ raise ObjectNotFound(name) from None
 
  def move(self, curr_name, new_name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  curr_path = self._validate_join(curr_name)
  new_path = self._validate_join(new_name)
  try:
  new_path.parent.mkdir(parents=True, exist_ok=True)
  curr_path.replace(new_path)
  except FileNotFoundError:
- raise KeyError(curr_name) from None
+ raise ObjectNotFound(curr_name) from None
 
  def list(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  path = self._validate_join(name)
  try:
  for p in path.iterdir():
@@ -135,4 +160,4 @@ def list(self, name):
  size = 0 if is_dir else st.st_size
  yield ItemInfo(name=p.name, exists=True, size=size, directory=is_dir)
  except FileNotFoundError:
- raise KeyError(name) from None
+ raise ObjectNotFound(name) from None

src/borgstore/backends/sftp.py

@@ -9,6 +9,7 @@
 import paramiko
 
 from ._base import BackendBase, ItemInfo, validate_name
+from .errors import BackendMustBeOpen, BackendMustNotBeOpen, BackendDoesNotExist, BackendAlreadyExists, ObjectNotFound
 from ..constants import TMP_SUFFIX
 
 
@@ -46,10 +47,12 @@ def _disconnect(self):
 
  def create(self):
  if self.opened:
- raise self.MustNotBeOpen()
+ raise BackendMustNotBeOpen()
  self._connect()
  try:
- self._mkdir(self.base_path, parents=True, exist_ok=False)
+ self._mkdir(self.base_path, parents=False, exist_ok=False)
+ except (FileExistsError, IOError):
+ raise BackendAlreadyExists(f"sftp storage base path already exists: {self.base_path}")
  finally:
  self._disconnect()
 
@@ -65,26 +68,28 @@ def delete_recursive(path):
  self.client.rmdir(str(parent))
 
  if self.opened:
- raise self.MustNotBeOpen()
+ raise BackendMustNotBeOpen()
  self._connect()
  try:
  delete_recursive(self.base_path)
+ except FileNotFoundError:
+ raise BackendDoesNotExist(f"sftp storage base path does not exist: {self.base_path}")
  finally:
  self._disconnect()
 
  def open(self):
  if self.opened:
- raise self.MustNotBeOpen()
+ raise BackendMustNotBeOpen()
  self._connect()
  st = self.client.stat(self.base_path) # check if this storage exists, fail early if not.
  if not stat.S_ISDIR(st.st_mode):
- raise TypeError(f"sftp storage base path is not a directory: {self.base_path}")
+ raise BackendDoesNotExist(f"sftp storage base path is not a directory: {self.base_path}")
  self.client.chdir(self.base_path) # this sets the cwd we work in!
  self.opened = True
 
  def close(self):
  if not self.opened:
- raise self.MustBeOpen()
+ raise BackendMustBeOpen()
  self._disconnect()
  self.opened = False
 
@@ -106,17 +111,23 @@ def _mkdir(self, name, *, parents=False, exist_ok=False):
  raise
 
  def mkdir(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  validate_name(name)
  self._mkdir(name, parents=True, exist_ok=True)
 
  def rmdir(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  validate_name(name)
  try:
  self.client.rmdir(name)
  except FileNotFoundError:
- raise KeyError(name) from None
+ raise ObjectNotFound(name) from None
 
  def info(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  validate_name(name)
  try:
  st = self.client.stat(name)
@@ -128,16 +139,20 @@ def info(self, name):
  return ItemInfo(name=name, exists=True, directory=is_dir, size=size)
 
  def load(self, name, *, size=None, offset=0):
+ if not self.opened:
+ raise BackendMustBeOpen()
  validate_name(name)
  try:
  with self.client.open(name) as f:
  f.seek(offset)
  f.prefetch(size) # speeds up the following read() significantly!
  return f.read(size)
  except FileNotFoundError:
- raise KeyError(name) from None
+ raise ObjectNotFound(name) from None
 
  def store(self, name, value):
+ if not self.opened:
+ raise BackendMustBeOpen()
  validate_name(name)
  tmp_dir = Path(name).parent
  self._mkdir(str(tmp_dir), parents=True, exist_ok=True)
@@ -155,13 +170,17 @@ def store(self, name, value):
  raise
 
  def delete(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  validate_name(name)
  try:
  self.client.unlink(name)
  except FileNotFoundError:
- raise KeyError(name) from None
+ raise ObjectNotFound(name) from None
 
  def move(self, curr_name, new_name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  validate_name(curr_name)
  validate_name(new_name)
  try:
@@ -173,9 +192,11 @@ def move(self, curr_name, new_name):
  try:
  self.client.rename(curr_name, new_name) # use .posix_rename ?
  except FileNotFoundError:
- raise KeyError(curr_name) from None
+ raise ObjectNotFound(curr_name) from None
 
  def list(self, name):
+ if not self.opened:
+ raise BackendMustBeOpen()
  validate_name(name)
  try:
  for st in self.client.listdir_attr(name):
@@ -184,4 +205,4 @@ def list(self, name):
  size = 0 if is_dir else st.st_size
  yield ItemInfo(name=st.filename, exists=True, size=size, directory=is_dir)
  except FileNotFoundError:
- raise KeyError(name) from None
+ raise ObjectNotFound(name) from None

src/borgstore/mstore.py

@@ -23,8 +23,7 @@
 from typing import Iterator, Optional
 
 from .utils.nesting import split_key
-from .backends._base import ItemInfo
-from .store import Store
+from .store import Store, ItemInfo, ObjectNotFound
 
 
 def create_bucket_map(buckets: list[int]) -> dict[int, list[int]]:
@@ -133,20 +132,20 @@ def info(self, name: str, *, deleted=False) -> ItemInfo:
  store = self.stores[store_idx]
  try:
  return store.info(name, deleted=deleted)
- except KeyError:
+ except ObjectNotFound:
  pass # TODO: we expected the key to be there, but it was not. fix that by storing it there.
  else:
- raise KeyError(name) # didn't find it in any store
+ raise ObjectNotFound(name) # didn't find it in any store
 
  def load(self, name: str, *, size=None, offset=0, deleted=False) -> bytes:
  for store_idx in self._find_stores(name, mode="r"):
  store = self.stores[store_idx]
  try:
  return store.load(name, size=size, offset=offset, deleted=deleted)
- except KeyError:
+ except ObjectNotFound:
  pass # TODO: we expected the key to be there, but it was not. fix that by storing it there.
  else:
- raise KeyError(name) # didn't find it in any store
+ raise ObjectNotFound(name) # didn't find it in any store
 
  def store(self, name: str, value: bytes) -> None:
  for store_idx in self._find_stores(name, mode="w"):
@@ -158,7 +157,7 @@ def delete(self, name: str, *, deleted=False) -> None:
  store = self.stores[store_idx]
  try:
  store.delete(name, deleted=deleted)
- except KeyError:
+ except ObjectNotFound:
  pass # ignore it if it is already gone
 
  def move(
@@ -188,7 +187,7 @@ def move(
  if not new_name:
  raise ValueError("generic move needs new_name to be given.")
  store.move(name, new_name, deleted=deleted)
- except KeyError:
+ except ObjectNotFound:
  pass # ignore it, if it is not present in this store
 
  def list(self, name: str, deleted: bool = False) -> Iterator[ItemInfo]:

src/borgstore/store.py

@@ -6,6 +6,7 @@
 
 from .utils.nesting import nest
 from .backends._base import ItemInfo, BackendBase
+from .backends.errors import ObjectNotFound # noqa
 from .backends.posixfs import get_file_backend
 from .backends.sftp import get_sftp_backend
 from .constants import DEL_SUFFIX