Merge pull request #228 from reef-technologies/remove-command-errors

cleanup sync errors related to directories

Author: ppolewicz

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Add `get_bucket_name_or_none_from_bucket_id` to `AccountInfo` and `Cache`
 
 ### Fixed
+* Cleanup sync errors related to directories
 * Use proper error handling in `ScanPoliciesManager`
 
 ### Changed

b2sdk/_v2/exception.py

@@ -30,7 +30,6 @@
 from b2sdk.exception import CapExceeded
 from b2sdk.exception import ChecksumMismatch
 from b2sdk.exception import ClockSkew
-from b2sdk.exception import CommandError
 from b2sdk.exception import Conflict
 from b2sdk.exception import ConnectionReset
 from b2sdk.exception import DestFileNewer
@@ -67,9 +66,12 @@
 from b2sdk.exception import SSECKeyError
 from b2sdk.exception import WrongEncryptionModeForBucketDefault
 from b2sdk.exception import interpret_b2_error
+from b2sdk.sync.exception import EmptyDirectory
 from b2sdk.sync.exception import EnvironmentEncodingError
 from b2sdk.sync.exception import IncompleteSync
 from b2sdk.sync.exception import InvalidArgument
+from b2sdk.sync.exception import NotADirectory
+from b2sdk.sync.exception import UnableToCreateDirectory
 from b2sdk.sync.exception import UnSyncableFilename
 from b2sdk.sync.exception import check_invalid_argument
 
@@ -94,12 +96,12 @@
     'CapExceeded',
     'ChecksumMismatch',
     'ClockSkew',
-    'CommandError',
     'Conflict',
     'ConnectionReset',
     'CorruptAccountInfo',
     'DestFileNewer',
     'DuplicateBucketName',
+    'EmptyDirectory',
     'EnvironmentEncodingError',
     'FileAlreadyHidden',
     'FileNameNotAllowed',
@@ -116,6 +118,7 @@
     'MissingAccountData',
     'MissingPart',
     'NonExistentBucket',
+    'NotADirectory',
     'NotAllowedByAppKeyError',
     'PartSha1Mismatch',
     'RestrictedBucket',
@@ -130,6 +133,7 @@
     'UnknownError',
     'UnknownHost',
     'UnrecognizedBucketType',
+    'UnableToCreateDirectory',
     'UnSyncableFilename',
     'UnsatisfiableRange',
     'UnusableFileName',

b2sdk/exception.py

@@ -172,21 +172,6 @@ def __str__(self):
             )
 
 
-class CommandError(B2Error):
-    """
-    b2 command error (user caused).  Accepts exactly one argument: message.
-
-    We expect users of shell scripts will parse our ``__str__`` output.
-    """
-
-    def __init__(self, message):
-        super(CommandError, self).__init__()
-        self.message = message
-
-    def __str__(self):
-        return self.message
-
-
 class Conflict(B2SimpleError):
     pass
 

b2sdk/sync/exception.py

@@ -89,3 +89,21 @@ def check_invalid_argument(parameter_name: str, message: str,
         if not message:
             message = str(exc)
         raise InvalidArgument(parameter_name, message) from exc
+
+
+class BaseDirectoryError(B2SimpleError):
+    def __init__(self, path):
+        self.path = path
+        super().__init__(path)
+
+
+class EmptyDirectory(BaseDirectoryError):
+    pass
+
+
+class UnableToCreateDirectory(BaseDirectoryError):
+    pass
+
+
+class NotADirectory(BaseDirectoryError):
+    pass

b2sdk/sync/folder.py

@@ -15,8 +15,7 @@
 import sys
 
 from abc import ABCMeta, abstractmethod
-from b2sdk.exception import CommandError
-from .exception import EnvironmentEncodingError, UnSyncableFilename
+from .exception import EmptyDirectory, EnvironmentEncodingError, UnSyncableFilename, NotADirectory, UnableToCreateDirectory
 from .file import File, B2File, FileVersion, B2FileVersion
 from .scan_policies import DEFAULT_SCAN_MANAGER
 from ..utils import fix_windows_path_limit, get_file_mtime, is_file_readable
@@ -166,9 +165,9 @@ def ensure_present(self):
             try:
                 os.mkdir(self.root)
             except OSError:
-                raise Exception('unable to create directory %s' % (self.root,))
+                raise UnableToCreateDirectory(self.root)
         elif not os.path.isdir(self.root):
-            raise Exception('%s is not a directory' % (self.root,))
+            raise NotADirectory(self.root)
 
     def ensure_non_empty(self):
         """
@@ -177,9 +176,7 @@ def ensure_non_empty(self):
         self.ensure_present()
 
         if not os.listdir(self.root):
-            raise CommandError(
-                'Directory %s is empty.  Use --allowEmptySource to sync anyway.' % (self.root,)
-            )
+            raise EmptyDirectory(self.root)
 
     @classmethod
     def _walk_relative_paths(cls, local_dir, b2_dir, reporter, policies_manager):

b2sdk/sync/folder_parser.py

@@ -8,7 +8,7 @@
 #
 ######################################################################
 
-from ..exception import CommandError
+from .exception import InvalidArgument
 from .folder import B2Folder, LocalFolder
 
 
@@ -44,7 +44,7 @@ def _parse_bucket_and_folder(bucket_and_path, api, b2_folder_class):
     Turn 'my-bucket/foo' into B2Folder(my-bucket, foo).
     """
     if '//' in bucket_and_path:
-        raise CommandError("'//' not allowed in path names")
+        raise InvalidArgument('folder_name', "'//' not allowed in path names")
     if '/' not in bucket_and_path:
         bucket_name = bucket_and_path
         folder_name = ''

b2sdk/v1/exception.py

@@ -9,3 +9,19 @@
 ######################################################################
 
 from b2sdk._v2.exception import *  # noqa
+
+
+# This exception class is deprecated and should not be used in new designs
+class CommandError(B2Error):
+    """
+    b2 command error (user caused).  Accepts exactly one argument: message.
+
+    We expect users of shell scripts will parse our ``__str__`` output.
+    """
+
+    def __init__(self, message):
+        super(CommandError, self).__init__()
+        self.message = message
+
+    def __str__(self):
+        return self.message

b2sdk/v1/sync/folder.py

@@ -9,9 +9,28 @@
 ######################################################################
 
 from abc import abstractmethod
+import functools
 
 from b2sdk import _v2 as v2
 from .scan_policies import DEFAULT_SCAN_MANAGER
+from .. import exception
+
+
+def translate_errors(func):
+    @functools.wraps(func)
+    def wrapper(*a, **kw):
+        try:
+            return func(*a, **kw)
+        except exception.NotADirectory as ex:
+            raise Exception('%s is not a directory' % (ex.path,))
+        except exception.UnableToCreateDirectory as ex:
+            raise Exception('unable to create directory %s' % (ex.path,))
+        except exception.EmptyDirectory as ex:
+            raise exception.CommandError(
+                'Directory %s is empty.  Use --allowEmptySource to sync anyway.' % (ex.path,)
+            )
+
+    return wrapper
 
 
 # Override to change "policies_manager" default argument
@@ -21,9 +40,16 @@ def all_files(self, reporter, policies_manager=DEFAULT_SCAN_MANAGER):
         pass
 
 
-class LocalFolder(v2.LocalFolder, AbstractFolder):
+class B2Folder(v2.B2Folder, AbstractFolder):
     pass
 
 
-class B2Folder(v2.B2Folder, AbstractFolder):
-    pass
+# "policies_manager" default argument and translate nice errors to old style Exceptions and CommandError
+class LocalFolder(v2.LocalFolder, AbstractFolder):
+    @translate_errors
+    def ensure_present(self):
+        return super().ensure_present()
+
+    @translate_errors
+    def ensure_non_empty(self):
+        return super().ensure_non_empty()

b2sdk/v1/sync/folder_parser.py

@@ -9,10 +9,14 @@
 ######################################################################
 
 from b2sdk import _v2 as v2
+from .. import exception
 
 from .folder import LocalFolder, B2Folder
 
 
-# Override to use v1 version of "LocalFolder" and "B2Folder"
+# Override to use v1 version of "LocalFolder" and "B2Folder" and raise old style CommandError
 def parse_sync_folder(folder_name, api):
-    return v2.parse_sync_folder(folder_name, api, LocalFolder, B2Folder)
+    try:
+        return v2.parse_sync_folder(folder_name, api, LocalFolder, B2Folder)
+    except exception.InvalidArgument as ex:
+        raise exception.CommandError(ex.message)

test/unit/sync/test_sync.py

@@ -13,7 +13,7 @@
 from functools import partial
 
 from apiver_deps import B2DownloadAction, B2UploadAction, B2CopyAction, AbstractSyncEncryptionSettingsProvider, UploadSourceLocalFile
-from apiver_deps_exception import CommandError, DestFileNewer, InvalidArgument
+from apiver_deps_exception import DestFileNewer, InvalidArgument
 from b2sdk.utils import TempDir
 
 from .fixtures import *
@@ -50,6 +50,7 @@ def assert_folder_sync_actions(self, synchronizer, src_folder, dst_folder, expec
         )
         assert expected_actions == [str(a) for a in actions]
 
+    @pytest.mark.apiver(to_ver=0)
     @pytest.mark.parametrize(
         'args', [
             {
@@ -64,10 +65,28 @@ def assert_folder_sync_actions(self, synchronizer, src_folder, dst_folder, expec
             'keep_days_or_delete',
         ]
     )
-    def test_illegal_args(self, synchronizer_factory, apiver, args):
-        exceptions = defaultdict(lambda: InvalidArgument, v0=CommandError)  # noqa
+    def test_illegal_args_up_to_v0(self, synchronizer_factory, apiver, args):
+        from apiver_deps_exception import CommandError
+        with pytest.raises(CommandError):
+            synchronizer_factory(**args)
 
-        with pytest.raises(exceptions[apiver]):
+    @pytest.mark.apiver(from_ver=1)
+    @pytest.mark.parametrize(
+        'args', [
+            {
+                'newer_file_mode': IllegalEnum.ILLEGAL
+            },
+            {
+                'keep_days_or_delete': IllegalEnum.ILLEGAL
+            },
+        ],
+        ids=[
+            'newer_file_mode',
+            'keep_days_or_delete',
+        ]
+    )
+    def test_illegal_args_up_v1_and_up(self, synchronizer_factory, apiver, args):
+        with pytest.raises(InvalidArgument):
             synchronizer_factory(**args)
 
     def test_illegal(self, synchronizer):

test/unit/test_exception.py

@@ -8,14 +8,15 @@
 #
 ######################################################################
 
+import pytest
+
 from b2sdk.exception import FileOrBucketNotFound, ResourceNotFound
 from apiver_deps_exception import (
     AlreadyFailed,
     B2Error,
     BadJson,
     BadUploadUrl,
     CapExceeded,
-    CommandError,
     Conflict,
     DuplicateBucketName,
     FileAlreadyHidden,
@@ -56,7 +57,9 @@ def test_already_failed_exception(self):
         except AlreadyFailed as e:
             assert str(e) == 'Already failed: foo', str(e)
 
+    @pytest.mark.apiver(to_ver=1)
     def test_command_error(self):
+        from apiver_deps_exception import CommandError
         try:
             raise CommandError('foo')
         except CommandError as e: