Idea #14965
closedPort arv-mount to Python 3
Added by Tom Morris over 5 years ago. Updated about 5 years ago.
Related issues
Updated by Tom Morris over 5 years ago
- Tracker changed from Task to Idea
- Target version changed from Arvados Future Sprints to To Be Groomed
Updated by Tom Morris over 5 years ago
- Related to Idea #14532: [Epic] Port to Python 3 to for Python 2 sunset in December 2019 added
Updated by Tom Morris over 5 years ago
- Has duplicate Bug #15251: arv-mount supports python 3 added
Updated by Tom Morris about 5 years ago
- Target version changed from To Be Groomed to Arvados Future Sprints
- Story points set to 2.0
Updated by Eric Biagiotti about 5 years ago
- Assigned To set to Eric Biagiotti
- Target version changed from Arvados Future Sprints to 2019-06-19 Sprint
Updated by Eric Biagiotti about 5 years ago
- Status changed from New to In Progress
Updated by Eric Biagiotti about 5 years ago
- Target version changed from 2019-06-19 Sprint to 2019-07-03 Sprint
Updated by Eric Biagiotti about 5 years ago
- Target version changed from 2019-07-03 Sprint to 2019-07-17 Sprint
Updated by Eric Biagiotti about 5 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:
- https://ci.curoverse.com/view/Developer/job/developer-run-tests/1400/
- Also Manually tested arv-mount installation and some functionality in py3/py2 virtualenvs
Updated by Eric Biagiotti about 5 years ago
- Target version changed from 2019-07-17 Sprint to Arvados Future Sprints
Updated by Eric Biagiotti about 5 years ago
- Target version changed from Arvados Future Sprints to 2019-07-31 Sprint
Updated by Lucas Di Pentima about 5 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 somedict.itervalues()
that were translated todict.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 callingitervalues(dict)
?services/fuse/arvados_fuse/__init__.py:L276
— Usingdict.items()
is inefficient on Py2 as described on the previously mentioned guide list, maybe useviewitems(dict)
oriteritems(dict)
?services/fuse/arvados_fuse/fusedir.py:L99
andservices/fuse/arvados_fuse/__init__.py:L727
— Usinglist(dict.items())
is inefficient on Py2 as described on the previously mentioned guide list, maybe uselistitems(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
Updated by Eric Biagiotti about 5 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.
- 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)
Thanks for finding that! Fixed in latest.
services/fuse/arvados_fuse/fusedir.py
- Lines 170 & 178: There’re somedict.itervalues()
that were translated todict.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 callingitervalues(dict)
?services/fuse/arvados_fuse/__init__.py:L276
— Usingdict.items()
is inefficient on Py2 as described on the previously mentioned guide list, maybe useviewitems(dict)
oriteritems(dict)
?services/fuse/arvados_fuse/fusedir.py:L99
andservices/fuse/arvados_fuse/__init__.py:L727
— Usinglist(dict.items())
is inefficient on Py2 as described on the previously mentioned guide list, maybe uselistitems(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
Updated by Lucas Di Pentima about 5 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!
Updated by Eric Biagiotti about 5 years ago
- Status changed from In Progress to Resolved