-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
s3: provide more helpful messages on common errors #4480
Conversation
@@ -162,7 +162,7 @@ class RelPath(str): | |||
"endpointurl": str, | |||
"access_key_id": str, | |||
"secret_access_key": str, | |||
Optional("listobjects", default=False): Bool, | |||
Optional("listobjects", default=False): Bool, # obsoleted |
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.
Obsoleted since we are using a high-level resource API now, which just does the switch automatically.
@@ -75,24 +73,52 @@ def s3(self): | |||
|
|||
session = boto3.session.Session(**session_opts) | |||
|
|||
return session.client( | |||
return session.resource( |
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.
resource
is a higher level API, so we don't have to manually parse-out HTTP codes.
try: | ||
yield bucket.Object(path_info.path) | ||
except bucket.meta.client.exceptions.NoSuchKey as exc: | ||
raise DvcException(f"{path_info.url} does not exist") from exc |
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.
We'll move to FileNotFoundError in the future, but for now just doing the same thing that we used to.
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Fix #4478
β I have followed the Contributing to DVC checklist.
π If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. π