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
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
Updated by Tom Clegg about 8 years ago
- Assigned To changed from Tom Morris to Tom Clegg
10014-collection-error-detail @ f64b139
Updated by Lucas Di Pentima about 8 years ago
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?
Updated by Tom Clegg about 8 years ago
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 ofcreate_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
Updated by Tom Clegg about 8 years ago
- Category set to SDKs
- Target version set to 2016-09-28 sprint
Updated by Tom Clegg about 8 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|commit:8cb96c1b23d02be8fac545d8c05a167454c1a11d.