Project

General

Profile

Actions

Idea #14965

closed

Port arv-mount to Python 3

Added by Tom Morris about 5 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Eric Biagiotti
Category:
-
Target version:
Story points:
2.0
Release relationship:
Auto

Subtasks 1 (0 open1 closed)

Task #15328: Review 14965-arv-mount-py-threeResolvedLucas Di Pentima07/16/2019Actions

Related issues

Related to Arvados Epics - Idea #14532: [Epic] Port to Python 3 to for Python 2 sunset in December 2019Resolved01/01/202009/16/2020Actions
Has duplicate Arvados - Bug #15251: arv-mount supports python 3DuplicateActions
Actions #1

Updated by Tom Morris almost 5 years ago

  • Parent task deleted (#14532)
Actions #2

Updated by Tom Morris almost 5 years ago

  • Tracker changed from Task to Idea
  • Target version changed from Arvados Future Sprints to To Be Groomed
Actions #3

Updated by Tom Morris almost 5 years ago

  • Related to Idea #14532: [Epic] Port to Python 3 to for Python 2 sunset in December 2019 added
Actions #4

Updated by Tom Morris almost 5 years ago

  • Has duplicate Bug #15251: arv-mount supports python 3 added
Actions #5

Updated by Tom Morris almost 5 years ago

  • Target version changed from To Be Groomed to Arvados Future Sprints
  • Story points set to 2.0
Actions #6

Updated by Eric Biagiotti almost 5 years ago

  • Assigned To set to Eric Biagiotti
  • Target version changed from Arvados Future Sprints to 2019-06-19 Sprint
Actions #7

Updated by Ward Vandewege almost 5 years ago

  • Release set to 22
Actions #8

Updated by Eric Biagiotti almost 5 years ago

  • Status changed from New to In Progress
Actions #9

Updated by Eric Biagiotti almost 5 years ago

  • Target version changed from 2019-06-19 Sprint to 2019-07-03 Sprint
Actions #10

Updated by Eric Biagiotti over 4 years ago

  • Target version changed from 2019-07-03 Sprint to 2019-07-17 Sprint
Actions #11

Updated by Eric Biagiotti over 4 years ago

Latest at 16d9c611d5026e50692e1b3d0fefb951de5afa7b

  • arv-mount supports Python 3
  • Besides the futurize changes, I mostly fixed test encoding issues. I tried to minimize changes to our actual fuse code.

Testing:

Actions #12

Updated by Eric Biagiotti over 4 years ago

  • Target version changed from 2019-07-17 Sprint to Arvados Future Sprints
Actions #13

Updated by Eric Biagiotti over 4 years ago

  • Target version changed from Arvados Future Sprints to 2019-07-31 Sprint
Actions #14

Updated by Lucas Di Pentima over 4 years ago

Sorry for the delay, I was doing some manual testing on arvbox and it took me some time.

As we have had performance issues in the past with the fuse driver, some of my comments are headed towards that, following this Python 2/3 "cheat sheet": http://python-future.org/compatible_idioms.html

  • When running tests, I’m getting lots of warning messages like this one: /usr/lib/python3.5/json/decoder.py:355: ResourceWarning: unclosed <ssl.SSLSocket fd=21, family=AddressFamily.AF_INET, type=2049, proto=6, laddr=('127.0.0.1', 42742), raddr=('127.0.0.1', 47393)> — as we talked on zoom, I’m worried about resources not being fully freed after a test run (may create issues on Jenkins slaves), but if that’s not the case, I think it would still be good to avoid them for log readability purposes.
  • Also got some deprecation warnings: arvados/services/fuse/tests/test_exec.py:65: DeprecationWarning: Please use assertRegex instead. (check https://six.readthedocs.io/#unittest-assertions for py2/py3 compatibility)
  • services/fuse/arvados_fuse/fusedir.py - Lines 170 & 178: There’re some dict.itervalues() that were translated to dict.values() calls, that would use extra memory on python2, as it’s described on http://python-future.org/compatible_idioms.html#iterating-through-dict-keys-values-items, seems that the way to go is calling itervalues(dict)?
  • services/fuse/arvados_fuse/__init__.py:L276 — Using dict.items() is inefficient on Py2 as described on the previously mentioned guide list, maybe use viewitems(dict) or iteritems(dict)?
  • services/fuse/arvados_fuse/fusedir.py:L99 and services/fuse/arvados_fuse/__init__.py:L727 — Using list(dict.items()) is inefficient on Py2 as described on the previously mentioned guide list, maybe use listitems(dict)?

As for the manual tests, at first I initially got my arvbox shell instance to freeze when trying to write a file using py3 after trying it first with py2, but later on I did a proper test initially starting a shell and trying py2 and later closing the shell and re-starting it and doing the same with py3, and it worked without issues, so maybe I didn't unmounted the filesystem correctly at the first try.

Also, I think the documentation needs updating to state that it's compatible with Python3 https://doc.arvados.org/sdk/python/arvados-fuse.html

Actions #15

Updated by Eric Biagiotti over 4 years ago

Latest at 3ce47c5d012f9dbd6335ad0ae092eb9cc8a3022e

Unit tests: https://ci.curoverse.com/job/developer-run-tests/1404/ - Some workbench integration failures, but I ran them locally and they passed.
Conformance tests: https://ci.curoverse.com/view/CWL/job/arvados-cwl-conformance-tests/193/

Also retested manually in virtualenvs.

Lucas Di Pentima wrote:

  • When running tests, I’m getting lots of warning messages like this one: /usr/lib/python3.5/json/decoder.py:355: ResourceWarning: unclosed <ssl.SSLSocket fd=21, family=AddressFamily.AF_INET, type=2049, proto=6, laddr=('127.0.0.1', 42742), raddr=('127.0.0.1', 47393)> — as we talked on zoom, I’m worried about resources not being fully freed after a test run (may create issues on Jenkins slaves), but if that’s not the case, I think it would still be good to avoid them for log readability purposes.

I was able to reduce some of these with a1a1a8756149d5d05526503fd1e85115ea083fac, but the json and yaml ones have been difficult to pin down. I'm fairly certain these pertain to http2libs keep-alive feature https://github.com/httplib2/httplib2#keep-alive. Similar issues have been reported here https://github.com/kennethreitz/requests/issues/3912. The reason these only show up in testing is because they are normally suppressed by default in python, but unittest resets warning suppression.

They don't seem to be causing problems, but their impact on jenkins slaves is worth keeping in mind. It's also worth noting that these warnings show up in majority of our py3 test suites (sdk/python, sdk/cwl). For these reasons, I would like to leave the output as is and make a separate issue to investigate further. Let me know if you think this is satisfactory and I will make the issue.

Thanks for finding that! Fixed in latest.

  • services/fuse/arvados_fuse/fusedir.py - Lines 170 & 178: There’re some dict.itervalues() that were translated to dict.values() calls, that would use extra memory on python2, as it’s described on http://python-future.org/compatible_idioms.html#iterating-through-dict-keys-values-items, seems that the way to go is calling itervalues(dict)?
  • services/fuse/arvados_fuse/__init__.py:L276 — Using dict.items() is inefficient on Py2 as described on the previously mentioned guide list, maybe use viewitems(dict) or iteritems(dict)?
  • services/fuse/arvados_fuse/fusedir.py:L99 and services/fuse/arvados_fuse/__init__.py:L727 — Using list(dict.items()) is inefficient on Py2 as described on the previously mentioned guide list, maybe use listitems(dict)?

Fixed all three in latest.

Also, I think the documentation needs updating to state that it's compatible with Python3 https://doc.arvados.org/sdk/python/arvados-fuse.html

That page will be updated once we create the py3 arv-mount package. I'm tracking it here: https://dev.arvados.org/issues/15473

Actions #16

Updated by Lucas Di Pentima over 4 years ago

Eric Biagiotti wrote:

Latest at 3ce47c5d012f9dbd6335ad0ae092eb9cc8a3022e
[...]
They don't seem to be causing problems, but their impact on jenkins slaves is worth keeping in mind. It's also worth noting that these warnings show up in majority of our py3 test suites (sdk/python, sdk/cwl). For these reasons, I would like to leave the output as is and make a separate issue to investigate further. Let me know if you think this is satisfactory and I will make the issue.

Yes, thanks for doing the research! I wasn't aware of the warnings happening on other py3 test runs.

[...]

Also, I think the documentation needs updating to state that it's compatible with Python3 https://doc.arvados.org/sdk/python/arvados-fuse.html

That page will be updated once we create the py3 arv-mount package. I'm tracking it here: https://dev.arvados.org/issues/15473

Great, so this LGTM, thanks!

Actions #18

Updated by Eric Biagiotti over 4 years ago

  • Status changed from In Progress to Resolved
Actions

Also available in: Atom PDF