-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exceptions #131
Exceptions #131
Conversation
Codecov Report
@@ Coverage Diff @@
## master #131 +/- ##
========================================
+ Coverage 92.2% 93.5% +1.3%
========================================
Files 20 21 +1
Lines 1098 1119 +21
========================================
+ Hits 1013 1047 +34
+ Misses 85 72 -13
|
@@ -29,7 +29,8 @@ | |||
from .mock_clients.mock_s3 import mocked_session_class_factory | |||
|
|||
|
|||
load_dotenv(find_dotenv()) | |||
if os.getenv("USE_LIVE_CLOUD") == "1": | |||
load_dotenv(find_dotenv()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This had fooled my local testing by loading environment variables when I wasn't expecting them for the mocked tests.
Ended up doing more on fixing tests than I expected, but this is actually ready now @pjbull |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, thanks for digging in and making this neat and tidy. Loving the readable, standardized exceptions
cloudpathlib
exceptions to be located in newcloudpathlib.exceptions
module.cloudpathlib
exceptions to subclass from new baseCloudPathException
. This allows for easy catching of any custom exception fromcloudpathlib
.Error
as recommended by PEP 8.CloudPathFileExistsError
,CloudPathIsADirectoryError
orCloudPathNotADirectoryError
exceptions instead of a genericValueError
.cloudpathlib
package namespace. Import fromcloudpathlib.exceptions
instead if needed.AzureBlobClient
instantiation to throw new errorMissingCredentialsError
when no credentials are provided, instead ofAttributeError
.LocalAzureBlobClient
changed to throw error under same conditions accordingly.GSClient
to instantiate as anonymous with public access only when instantiated with no credentials, instead of erroring.Some additional testing updates:
conftest.py
only runload_dotenv
ifUSE_LIVE_CLOUD
is set to 1. This was fooling my testing because an unset env var was present in my.env
file.cloudpathlib.local
equivalents.