diff --git a/CHANGES b/CHANGES index 6a3d769bf..59cf5d40f 100644 --- a/CHANGES +++ b/CHANGES @@ -8,6 +8,7 @@ Version 0.9 (feature release, released on X) +- Improved error handling / reporting. (#12) - Use fcntl() instead of flock() when locking repository/cache. (#15) - Let ssh figure out port/user if not specified so we don't override .ssh/config (#9) diff --git a/attic/archive.py b/attic/archive.py index 185d41ba7..4526fe099 100644 --- a/attic/archive.py +++ b/attic/archive.py @@ -10,7 +10,7 @@ import time from io import BytesIO from attic import xattr from attic.chunker import chunkify -from attic.helpers import uid2user, user2uid, gid2group, group2gid, \ +from attic.helpers import Error, uid2user, user2uid, gid2group, group2gid, \ Statistics, decode_dict, st_mtime_ns, make_path_safe ITEMS_BUFFER = 1024 * 1024 @@ -72,11 +72,11 @@ class ItemIter(object): class Archive(object): - class DoesNotExist(Exception): - pass + class DoesNotExist(Error): + """Archive {} does not exist""" - class AlreadyExists(Exception): - pass + class AlreadyExists(Error): + """Archive {} already exists""" def __init__(self, repository, key, manifest, name, cache=None, create=False, checkpoint_interval=300, numeric_owner=False): diff --git a/attic/archiver.py b/attic/archiver.py index f258a0152..0acd96463 100644 --- a/attic/archiver.py +++ b/attic/archiver.py @@ -11,7 +11,7 @@ from attic.archive import Archive from attic.repository import Repository from attic.cache import Cache from attic.key import key_creator -from attic.helpers import location_validator, format_time, \ +from attic.helpers import Error, location_validator, format_time, \ format_file_mode, ExcludePattern, exclude_path, adjust_patterns, to_localtime, \ get_cache_dir, get_keys_dir, format_timedelta, prune_split, Manifest, Location, remove_surrogates from attic.remote import RepositoryServer, RemoteRepository, ConnectionClosed @@ -477,24 +477,9 @@ def main(): archiver = Archiver() try: exit_code = archiver.run(sys.argv[1:]) - except Repository.DoesNotExist: - archiver.print_error('Error: Repository not found') - exit_code = 1 - except Repository.AlreadyExists: - archiver.print_error('Error: Repository already exists') - exit_code = 1 - except Archive.AlreadyExists as e: - archiver.print_error('Error: Archive "%s" already exists', e) - exit_code = 1 - except Archive.DoesNotExist as e: - archiver.print_error('Error: Archive "%s" does not exist', e) - exit_code = 1 - except Cache.RepositoryReplay: - archiver.print_error('Cache is newer than repository, refusing to continue') - exit_code = 1 - except ConnectionClosed: - archiver.print_error('Connection closed by remote host') - exit_code = 1 + except Error as e: + archiver.print_error(e.get_message()) + exit_code = e.exit_code except KeyboardInterrupt: archiver.print_error('Error: Keyboard interrupt') exit_code = 1 diff --git a/attic/cache.py b/attic/cache.py index 8766f9cc3..e1a475761 100644 --- a/attic/cache.py +++ b/attic/cache.py @@ -5,7 +5,7 @@ import os from binascii import hexlify import shutil -from .helpers import get_cache_dir, decode_dict, st_mtime_ns, unhexlify, UpgradableLock +from .helpers import Error, get_cache_dir, decode_dict, st_mtime_ns, unhexlify, UpgradableLock from .hashindex import ChunkIndex @@ -13,9 +13,8 @@ class Cache(object): """Client Side cache """ - class RepositoryReplay(Exception): - """ - """ + class RepositoryReplay(Error): + """Cache is newer than repository, refusing to continue""" def __init__(self, repository, key, manifest): self.timestamp = None diff --git a/attic/helpers.py b/attic/helpers.py index d0ff3607d..275714054 100644 --- a/attic/helpers.py +++ b/attic/helpers.py @@ -14,9 +14,22 @@ from operator import attrgetter import fcntl +class Error(Exception): + """Error base class""" + + exit_code = 1 + + def get_message(self): + return 'Error: ' + type(self).__doc__.format(*self.args) + + class UpgradableLock: + class LockUpgradeFailed(Error): + """Failed to acquire write lock on {}""" + def __init__(self, path, exclusive=False): + self.path = path try: self.fd = open(path, 'r+') except IOError: @@ -28,7 +41,10 @@ class UpgradableLock: self.is_exclusive = exclusive def upgrade(self): - fcntl.lockf(self.fd, fcntl.LOCK_EX) + try: + fcntl.lockf(self.fd, fcntl.LOCK_EX) + except OSError as e: + raise self.LockUpgradeFailed(self.path) self.is_exclusive = True def release(self): diff --git a/attic/remote.py b/attic/remote.py index 2eb384100..8338f73ee 100644 --- a/attic/remote.py +++ b/attic/remote.py @@ -4,17 +4,16 @@ import os import select from subprocess import Popen, PIPE import sys -import getpass +from .helpers import Error from .repository import Repository from .lrucache import LRUCache BUFSIZE = 10 * 1024 * 1024 -class ConnectionClosed(Exception): - """Connection closed by remote host - """ +class ConnectionClosed(Error): + """Connection closed by remote host""" class RepositoryServer(object): diff --git a/attic/repository.py b/attic/repository.py index eb65bd78d..bb4418093 100644 --- a/attic/repository.py +++ b/attic/repository.py @@ -7,7 +7,7 @@ import struct from zlib import crc32 from .hashindex import NSIndex -from .helpers import IntegrityError, read_msgpack, write_msgpack, unhexlify, UpgradableLock +from .helpers import Error, IntegrityError, read_msgpack, write_msgpack, unhexlify, UpgradableLock from .lrucache import LRUCache MAX_OBJECT_SIZE = 20 * 1024 * 1024 @@ -30,11 +30,15 @@ class Repository(object): DEFAULT_MAX_SEGMENT_SIZE = 5 * 1024 * 1024 DEFAULT_SEGMENTS_PER_DIR = 10000 - class DoesNotExist(KeyError): - """Requested key does not exist""" + class DoesNotExist(Error): + """Repository {} does not exist""" + + class AlreadyExists(Error): + """Repository {} already exists""" + + class InvalidRepository(Error): + """{} is not a valid repository""" - class AlreadyExists(KeyError): - """Requested key does not exist""" def __init__(self, path, create=False): self.io = None @@ -70,11 +74,11 @@ class Repository(object): self.path = path if not os.path.isdir(path): raise self.DoesNotExist(path) - self.lock = UpgradableLock(os.path.join(path, 'config')) self.config = RawConfigParser() self.config.read(os.path.join(self.path, 'config')) - if self.config.getint('repository', 'version') != 1: - raise Exception('%s Does not look like an Attic repository') + if not 'repository' in self.config.sections() or self.config.getint('repository', 'version') != 1: + raise self.InvalidRepository(path) + self.lock = UpgradableLock(os.path.join(path, 'config')) self.max_segment_size = self.config.getint('repository', 'max_segment_size') self.segments_per_dir = self.config.getint('repository', 'segments_per_dir') self.id = unhexlify(self.config.get('repository', 'id').strip()) diff --git a/attic/testsuite/helpers.py b/attic/testsuite/helpers.py index e1e7818bc..da0aff91a 100644 --- a/attic/testsuite/helpers.py +++ b/attic/testsuite/helpers.py @@ -84,5 +84,5 @@ class UpgradableLockTestCase(AtticTestCase): file = tempfile.NamedTemporaryFile() os.chmod(file.name, 0o444) lock = UpgradableLock(file.name) - self.assert_raises(OSError, lock.upgrade) + self.assert_raises(UpgradableLock.LockUpgradeFailed, lock.upgrade) lock.release()