Feature #10014
closed
[SDKs] Python SDK should be more forthcoming when a file can't be found in a collection: list the path that can't be found
Added by Ward Vandewege over 7 years ago.
Updated over 7 years ago.
Description
I propose a patch along these lines:
diff --git a/sdk/python/arvados/collection.py b/sdk/python/arvados/collection.py
index 56d8b23..0722f28 100644
--- a/sdk/python/arvados/collection.py
+++ b/sdk/python/arvados/collection.py
@@ -802,7 +802,7 @@ class RichCollectionBase(CollectionBase):
if isinstance(source, basestring):
source_obj = source_collection.find(source)
if source_obj is None:
- raise IOError(errno.ENOENT, "File not found")
+ raise IOError(errno.ENOENT, "File not found", source)
sourcecomponents = source.split("/")
else:
source_obj = source
- Description updated (diff)
- Description updated (diff)
- Description updated (diff)
- Assigned To set to Tom Morris
- Assigned To changed from Tom Morris to Tom Clegg
10014-collection-error-detail @ f64b139
- Status changed from New to In Progress
Grepping the
raise statements on
collection.py, I've found some additional spots that may be improved:
- Line 310: Would be useful to add the intended manifest stream that cannot be added because of containing a whitespace?
- Line 625: Read-only collection name can be added to the error message.
- Line 738: We refer to a “subcollection” not being empty, wouldn’t be more consistent to name it “directory” or “subdirectory” just like in other parts of the code?
- Line 818: Can target path and/or source be added to that ArgumentError message?
Also, a question related to find_or_create: If the file or collection exist and is not of create_type, shouldn’t we raise some exception instead of blindly return it without checking?
Lucas Di Pentima wrote:
- Line 310: Would be useful to add the intended manifest stream that cannot be added because of containing a whitespace?
Added.
- Line 625: Read-only collection name can be added to the error message.
Not actually sure how to get the collection name/uuid/pdh here...
- Line 738: We refer to a “subcollection” not being empty, wouldn’t be more consistent to name it “directory” or “subdirectory” just like in other parts of the code?
Yes, updated.
- Line 818: Can target path and/or source be added to that ArgumentError message?
Hm. The target path is empty by definition, and the intent seems to be to complain only if we don't have a string representation of source.
If I look closer, this function just starts to look rather buggy. sourcecomponents[-1] crashes before we even get to this error if source wasn't a basestring. Maybe investigate this in a separate branch?
Also, a question related to find_or_create: If the file or collection exist and is not of create_type, shouldn’t we raise some exception instead of blindly return it without checking?
The docstring claims this is the desired behavior but it does look like some callers are making different assumptions. Separate branch?
→commit:c727716
- Category set to SDKs
- Target version set to 2016-09-28 sprint
- Status changed from In Progress to Resolved
Applied in changeset arvados|commit:8cb96c1b23d02be8fac545d8c05a167454c1a11d.
Also available in: Atom
PDF
Updated by 
Updated by Tom Morris 
Updated by 
Updated by