Story #11419

[SDKs] support text-mode open() in Python 3

Added by Tom Clegg over 1 year ago. Updated about 2 hours ago.

Status:
Feedback
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
03/21/2017
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-

Subtasks

Task #14547: Review 11419-text-modeIn ProgressTom Clegg


Related issues

Related to Arvados - Story #11770: [Python SDK] Implement support for universal newline mode in Collections APINew2017-05-26

Related to Arvados - Story #11308: Support Python3 for arvados-python-client & command line utilitiesResolved2017-03-21

Blocks Arvados - Story #14532: Prepare for Python 2 sunsetting in December 2019New

Associated revisions

Revision 7317196f
Added by Tom Clegg 4 days ago

Merge branch '11419-text-mode'

closes #11419

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <>

History

#1 Updated by Tom Clegg over 1 year ago

  • Subject changed from open files in text or binary mode to support text-mode open() in Python 3

#2 Updated by Tom Clegg over 1 year ago

  • Subject changed from support text-mode open() in Python 3 to [SDKs] support text-mode open() in Python 3
  • Assigned To deleted (Tom Clegg)
  • Parent task deleted (#11308)

#3 Updated by Tom Clegg over 1 year ago

  • Tracker changed from Task to Feature
  • Target version deleted (2017-05-10 sprint)

#4 Updated by Tom Morris over 1 year ago

  • Target version set to Arvados Future Sprints

#5 Updated by Tom Morris 5 months ago

  • Related to Story #11308: Support Python3 for arvados-python-client & command line utilities added

#6 Updated by Tom Morris 5 months ago

  • Target version changed from Arvados Future Sprints to To Be Groomed

#7 Updated by Tom Morris 5 months ago

  • Release set to 14

#8 Updated by Tom Morris 5 months ago

  • Tracker changed from Feature to Story

#9 Updated by Tom Morris about 1 month ago

  • Release deleted (14)

#10 Updated by Tom Morris 18 days ago

  • Blocks Story #14532: Prepare for Python 2 sunsetting in December 2019 added

#11 Updated by Tom Morris 16 days ago

  • Target version changed from To Be Groomed to 2018-12-12 Sprint

#12 Updated by Tom Clegg 16 days ago

  • Assigned To set to Tom Clegg

#13 Updated by Tom Clegg 15 days ago

11419-text-mode @ 0565661b2c4fb220428ea37c2e0728e4b07d2465 https://ci.curoverse.com/view/Developer/job/developer-run-tests/989/

This change mostly comes down to wrapping the ArvadosFileReader/Writer in an io.BufferedReader/Random and then in an io.TextIOWrapper, if text mode is requested.

With the previous code, the "block boundary isn't on a character boundary" test crashed in Python 2 when using text mode, and the same wrapper fixes it, so I figured it would make sense to fix both at once. However, this does cause a slight API change that could conceivably break existing code. In text mode, the file-like object returned by Collection.open()...
  1. ...is not an ArvadosFileReader/Writer. Code that tests isinstance(f, ...) will behave differently. I found no examples of this in our own repo.
  2. ...cannot seek to arbitrary positions (see TextIOBase seek restrictions). The only occurrence of this I found in our repo was in the "open in text mode in Python 2" test.
  3. ...does not accept close(flush=False). This revealed (via sdk/cwl test failures) that source:sdk/python/arvados/commands/run.py was accidentally using text mode to copy arbitrary files.

#15 Updated by Tom Clegg 14 days ago

  • Status changed from New to In Progress

#17 Updated by Lucas Di Pentima 11 days ago

This LGTM, please merge.
Thanks!

#18 Updated by Tom Clegg 4 days ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

#19 Updated by Peter Amstutz about 2 hours ago

This has a regression:

https://ci.curoverse.com/view/CWL/job/run-cwl-test-4xphq/799/console

2018-12-14 20:28:39 arvados.cwl-runner ERROR: [step-valuefrom5-wf.cwl] While getting final output object: 'TextIOWrapper' object has no attribute 'size'
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/arvados_cwl/runner.py", line 465, in done
    if f.size() > 0:
AttributeError: 'TextIOWrapper' object has no attribute 'size'

#20 Updated by Peter Amstutz about 2 hours ago

  • Status changed from Resolved to Feedback
  • Target version changed from 2018-12-12 Sprint to 2018-12-21 Sprint

#21 Updated by Tom Clegg about 2 hours ago

11419-cwl-read-json-binary-mode @ fef817dc5d6d1e0160c6c14ba4f41581e241f33a

Also available in: Atom PDF