Story #11419

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

Added by Tom Clegg over 2 years ago. Updated 5 months ago.

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

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Subtasks

Task #14547: Review 11419-text-modeResolvedTom Clegg


Related issues

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

Related to Arvados - Story #11308: Support Python3 for arvados-python-client & command line utilitiesResolved03/21/2017

Blocks Arvados - Story #14532: [Epic] Port to Python 3 to prepare for Python 2 sunsetting in December 2019In Progress

Associated revisions

Revision 7317196f
Added by Tom Clegg 7 months ago

Merge branch '11419-text-mode'

closes #11419

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

Revision d7d074d7 (diff)
Added by Tom Clegg 7 months ago

11419: Read JSON files in binary mode.

Files opened in text mode do not have a size() method.

refs #11419

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

History

#1 Updated by Tom Clegg over 2 years ago

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

#2 Updated by Tom Clegg about 2 years 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 about 2 years ago

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

#4 Updated by Tom Morris almost 2 years ago

  • Target version set to Arvados Future Sprints

#5 Updated by Tom Morris 12 months ago

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

#6 Updated by Tom Morris 12 months ago

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

#7 Updated by Tom Morris 12 months ago

  • Release set to 14

#8 Updated by Tom Morris 12 months ago

  • Tracker changed from Feature to Story

#9 Updated by Tom Morris 8 months ago

  • Release deleted (14)

#10 Updated by Tom Morris 8 months ago

  • Blocks Story #14532: [Epic] Port to Python 3 to prepare for Python 2 sunsetting in December 2019 added

#11 Updated by Tom Morris 8 months ago

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

#12 Updated by Tom Clegg 8 months ago

  • Assigned To set to Tom Clegg

#13 Updated by Tom Clegg 8 months 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 8 months ago

  • Status changed from New to In Progress

#17 Updated by Lucas Di Pentima 7 months ago

This LGTM, please merge.
Thanks!

#18 Updated by Tom Clegg 7 months ago

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

#19 Updated by Peter Amstutz 7 months 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 7 months 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 7 months ago

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

#22 Updated by Tom Clegg 7 months ago

  • Status changed from Feedback to Resolved

#23 Updated by Tom Morris 5 months ago

  • Release set to 15

Also available in: Atom PDF