Bug #5074
closed[SDKs] CLI client and Ruby SDK should recover from (and avoid creating) unreadable discovery doc cache files.
Description
The current code creates a race condition while writing a file. It should create a temp file, write it, flush/close it, and rename it into place. If anything fails, it should delete the temp file.
Additionally, if a cache file exists (and is new enough, etc.) but is unreadable or unparseable, the SDK crashes. Instead, it should print a warning, then retrieve the discovery document from the server and continue.
See source:sdk/cli/bin/arv#L55 and source:sdk/ruby/lib/arvados.rb#L108
Bonus points: de-duplicate this copy-and-pasted code!
Updated by Tom Clegg almost 10 years ago
- Subject changed from [SDKs] Ruby SDK should recover from (and avoid creating) unreadable discovery doc cache files. to [SDKs] CLI client and Ruby SDK should recover from (and avoid creating) unreadable discovery doc cache files.
- Description updated (diff)
- Category set to SDKs
Updated by Brett Smith almost 10 years ago
- Status changed from New to In Progress
- Assigned To set to Brett Smith
- Target version changed from Bug Triage to 2015-01-28 Sprint
Updated by Peter Amstutz almost 10 years ago
The code looks good. I appreciate that you moved the logic to the Ruby SDK.
So I intentionally corrupted the cached discovery document and got this error:
Warning: error reading cached discovery document /home/peter/.cache/arvados/discovery-86e60ccc4da91c726aa80dfb73fa9cbf.json: 795: unexpected token at ':"discovery#restDescription","discoveryVersion":"v1","id":"arvados:v1","name":"arvados","version":"v1","revision":"20131114", "source_version":"1fae5d5","generatedAt":"2015-01-28T18:52:17+00:00","title":"Arvados API","description":"The API to interact with Arvados.","documentationLink":"http://doc.arvados.org/api/index.html","protocol":"rest","baseUrl":"https://qr1hi.arvadosapi.com/arvados/v1/", ... (and so forth for 1000s lines spewing the entire discover doc)
Consider not reporting an error in this case, because it is nonfatal and the user probably can't do anything with the information.
Does the Python discovery document cache code have the same robustness that the Ruby SDK will have with this branch?
Python appears to use a different caching scheme than Ruby, which means the discovery document gets stored twice?
Updated by Brett Smith almost 10 years ago
Peter Amstutz wrote:
Consider not reporting an error in this case, because it is nonfatal and the user probably can't do anything with the information.
Yeah, with 100-line error messages that's definitely not going to fly. Updated in d59deccc.
Does the Python discovery document cache code have the same robustness that the Ruby SDK will have with this branch?
The Python SDK lets httplib2 handle caching. I haven't dug into its source but I think we should trust them to deal with it robustly. They have to handle users a lot more demanding than us.
Thanks.
Updated by Brett Smith almost 10 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|commit:0fb26747fa229d6b19ec911b907259a8e84acd83.