Bug #11220
closed
[SDKs] Fix misleading arv-mount/pysdk error messages by removing obsolete "fetch manifest from Keep" code
Added by Tom Clegg about 7 years ago.
Updated over 6 years ago.
Description
Current error message:
2017-03-01 17:27:55 arvados.arvados_fuse[10741] ERROR: Error fetching collection '{{PDH}}': Failed to retrieve collection '{{PDH}}' from either API server (<HttpError 404 when requesting https://tb05z.arvadosapi.com/arvados/v1/collections/{{PDH}}?alt=json returned "Path not found">) or Keep ({{PDH}} not found: http://keep0.tb05z.arvadosapi.com:25107/ responded with 403 HTTP/1.1 403 Forbidden
This should be reported as 404 / not found. The 403 part is a red herring.
Manifests are no longer written to Keep, and even if they were, reading without a permission token will never work on most installations, so this fallback seems pointless.
source:sdk/python/arvados/collection.py#L1362
- Description updated (diff)
- Target version set to Arvados Future Sprints
- Target version changed from Arvados Future Sprints to 2017-10-25 Sprint
- Category set to SDKs
- Assigned To set to Tom Clegg
- Status changed from New to In Progress
11220-manifest-fetch-error @ 93e437b0dfd453f00df59c6a84bcc5d3ef09a9be
I removed one test that said "arv-get {block-id} -" relied on the fetch-manifest-from-Keep fallback. It looks like that's still true, and crunch-job calls arv-get that way to get manifest fragments from task outputs.
I implemented this a different way (so it also works for blocks that don't happen to be manifests) in commands/get.py, but it looks like only the manifest-from-api-server case is tested, not the data-from-keep case.
- Target version changed from 2017-10-25 Sprint to 2017-11-08 Sprint
- Ran tests both on Jenkins and locally, I got some
services/fuse failures: https://ci.curoverse.com/job/developer-run-tests/489/
Collection class constructor docstring should be updated to not mention block locators as a source of manifests.- The above comment also for
CollectionReader docstring.
- Shouldn't
Collection warn the developer with a deprecation notice when detected that manifest_locator is a block locator?
Lucas Di Pentima wrote:
Removed one (which tested for the behavior being removed) and fixed the other.
Collection class constructor docstring should be updated to not mention block locators as a source of manifests.- The above comment also for
CollectionReader docstring.
Updated.
- Shouldn't
Collection warn the developer with a deprecation notice when detected that manifest_locator is a block locator?
We could provide a more helpful error message/hint ("looks like you're trying to do ..., which is not supported any more") if the locator looks like a Keep locator (e.g., has a +A signature) -- is this what you have in mind?
11220-manifest-fetch-error @ 9dbeb1f31dafd927495ede39c7653b095495da26
Tom Clegg wrote:
We could provide a more helpful error message/hint ("looks like you're trying to do ..., which is not supported any more") if the locator looks like a Keep locator (e.g., has a +A signature) -- is this what you have in mind?
I think it would be helpful, although I'm not sure if the fact that manifests aren't saved on Keep is new enough to be worth the additional effort.
11220-manifest-fetch-error @ 9dbeb1f31dafd927495ede39c7653b095495da26
LGTM, thanks.
- Status changed from In Progress to Resolved
- % Done changed from 0 to 100
Applied in changeset arvados|commit:e67d0f5d43c56f78694ea4a5f93acec5c93cd0fb.
Also available in: Atom
PDF
Updated by 
Updated by Tom Morris 
Updated by 
Updated by Anonymous