Story #4316

Review 3603-pysdk-file-api-wip

Added by Brett Smith about 7 years ago. Updated about 7 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Brett Smith
Category:
-
Target version:
Start date:
08/27/2014
Due date:
% Done:

100%

Estimated time:
2.00 h
Story points:
0.5

Associated revisions

Revision 26aa2f1a
Added by Brett Smith about 7 years ago

Merge branch '3603-pysdk-file-api-wip'

Refs #3603. Closes #4316.

History

#1 Updated by Brett Smith about 7 years ago

  • Assigned To set to Peter Amstutz

#2 Updated by Brett Smith about 7 years ago

  • Story points set to 0.5

#3 Updated by Brett Smith about 7 years ago

  • Parent task deleted (#3603)

#4 Updated by Brett Smith about 7 years ago

  • Tracker changed from Task to Story
  • Target version changed from Arvados Future Sprints to 2014-11-19 sprint

#5 Updated by Brett Smith about 7 years ago

  • Assigned To changed from Peter Amstutz to Tim Pierce

#6 Updated by Brett Smith about 7 years ago

  • Assigned To changed from Tim Pierce to Brett Smith

Taking this back because I'm going to have to resolve some merge conflicts with a branch Tom is working on.

#7 Updated by Brett Smith about 7 years ago

  • Assigned To deleted (Brett Smith)

#8 Updated by Brett Smith about 7 years ago

  • Assigned To set to Peter Amstutz

#9 Updated by Peter Amstutz about 7 years ago

  • As discussed briefly in IRC, consider having slightly different behavior for StreamFileReader() based on how it is accessed. If accessed using existing Arvados methods, preserve existing behavior; if accessed using the new open() method, tweak the behavior to be maximally compatible as a Python file-like object.
    • For example, the behavior of seek()
  • Going down the documented API for file-like objects, there's a few small things that could be added:
    • A no-op flush() method
    • readline() takes an optional size parameter (maximum line length)
    • readlines() takes an optional size hint (approximate amount of total data to be read)
  • Regarding isatty():
    "If a file-like object is not associated with a real file, this method should not be implemented."

#10 Updated by Peter Amstutz about 7 years ago

  • Assigned To changed from Peter Amstutz to Brett Smith

#11 Updated by Brett Smith about 7 years ago

Peter Amstutz wrote:

  • As discussed briefly in IRC, consider having slightly different behavior for StreamFileReader() based on how it is accessed.

We reviewed existing Crunch scripts, and found none that are using seek, or obviously relying on the decompression behavior in readlines. We'll take this opportunity to make a clean break and have our API be compatible with Python's as much as possible. All kinds of stuff is using the name() method, so we need to preserve backwards compatibility there.

Implemented all of these. Even made flush() meaningful.

  • Regarding isatty():
    "If a file-like object is not associated with a real file, this method should not be implemented."

Thanks for catching that. Removed. Ready for another look at 409a1a9.

#12 Updated by Brett Smith about 7 years ago

  • Assigned To changed from Brett Smith to Peter Amstutz

#13 Updated by Peter Amstutz about 7 years ago

  • Assigned To changed from Peter Amstutz to Brett Smith

LGTM

#14 Updated by Brett Smith about 7 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:26aa2f1a1c8c9883beac1538c318279190f91c8a.

Also available in: Atom PDF