Bug #21934
closedarv-copy uses subprocess.run(capture_output=True) which is not in Python 3.6 (RHEL 8)
Description
In CentOS releases there is version 3.6 of Python3 which gives error to user:
arv-copy -v --project-uuid arkau-j7d0g-h01qmcy1nnsr2ut arkau-7fd4e-fovvurkh3otlpyk
2024-06-26 08:29:12 arvados.arv-copy692 ERROR: init() got an unexpected keyword argument 'capture_output'
Traceback (most recent call last):
File "/usr/share/python3/dist/python3-arvados-python-client/lib/python3.6/site-packages/arvados/commands/arv_copy.py", line 158, in main
result = copy_workflow(args.object_uuid, src_arv, dst_arv, args)
File "/usr/share/python3/dist/python3-arvados-python-client/lib/python3.6/site-packages/arvados/commands/arv_copy.py", line 335, in copy_workflow
capture_output=True, env=env)
File "/usr/lib64/python3.6/subprocess.py", line 423, in run
with Popen(*popenargs, **kwargs) as process:
TypeError: init() got an unexpected keyword argument 'capture_output'
In Debian release we see in same package that there is Python3 3.9 version.
This is normal - or by mistake and CentOS Python3 version needs to be updated?
Files
Updated by Brett Smith 6 months ago
- Subject changed from Python3 version inside CentOS 8 / 7 releases inside python3-arvados-python-client-2.7.3 -- issue with arv-copy to arv-copy uses subprocess.Popen(capture_output=True) which is not in Python 3.6 (RHEL 8)
Updated by Brett Smith 6 months ago
Hi Krzysztof,
The intention was that the Arvados 2.7 series should continue to support Python 3.6. We'll have to talk through our options for dealing with this.
In Arvados 3.0 we're planning to have our Python RPMs depend on the python39 appstream to get a newer version of the language. Would that cause any issues for you? Would it be a problem if we rolled a new set of 2.7.3 packages that made that switch?
Updated by Brett Smith 6 months ago
- Subject changed from arv-copy uses subprocess.Popen(capture_output=True) which is not in Python 3.6 (RHEL 8) to arv-copy uses subprocess.run(capture_output=True) which is not in Python 3.6 (RHEL 8)
Updated by Brett Smith 6 months ago
The older way to say this is:
proc = subprocess.Popen(…, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
stdout, stderr = proc.communicate()
But if we want to try something like this, we should make sure to run all our Python test suites on Python 3.6 to make sure we're not overlooking anything else.
Updated by Krzysztof Dziedzic 6 months ago
User is using that inside shell node. We believe it would be ok to have Python 3.9 in rolled new set of 2.7.3.
We will start then with testing on ARDEV - but this upgrade from 3.6 to 3.9 should fix user issue.
Updated by Brett Smith 6 months ago
Other errors when running our test suite in Python 3.6. Note that the last one is strictly in the tests, not the SDK per se.
====================================================================== ERROR: test_disk_cache_retry_write_error (tests.test_keep_client.KeepDiskCacheTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/brett/arvados/sdk/python/tests/test_keep_client.py", line 1670, in test_disk_cache_retry_write_error self.assertIsNotNone(keep_client.get_from_cache(self.locator)) File "/home/brett/arvados/sdk/python/arvados/keep.py", line 1119, in get_from_cache return slot.get() File "/home/brett/arvados/sdk/python/arvados/diskcache.py", line 43, in get self.content.madvise(mmap.MADV_WILLNEED) AttributeError: 'mmap.mmap' object has no attribute 'madvise' ====================================================================== ERROR: test_initial_retry_logs (tests.test_api.ArvadosApiTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/httplib2/__init__.py", line 1349, in _conn_request conn.connect() File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/httplib2/__init__.py", line 1125, in connect address_info = socket.getaddrinfo(host, port, 0, socket.SOCK_STREAM) File "/var/lib/arvados/lib/python3.6/socket.py", line 745, in getaddrinfo for res in _socket.getaddrinfo(host, port, family, type, proto, flags): socket.gaierror: [Errno -2] Name or service not known During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/brett/arvados/sdk/python/arvados/api.py", line 102, in _intercept_http_request response, body = self.orig_http_request(uri, method, headers=headers, **kwargs) File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/httplib2/__init__.py", line 1712, in request conn, authority, uri, request_uri, method, body, headers, redirections, cachekey, File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/httplib2/__init__.py", line 1427, in _request (response, content) = self._conn_request(conn, request_uri, method, body, headers) File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/httplib2/__init__.py", line 1356, in _conn_request raise ServerNotFoundError("Unable to find the server at %s" % conn.host) httplib2.error.ServerNotFoundError: Unable to find the server at test.invalid During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/brett/arvados/sdk/python/arvados/api.py", line 103, in _intercept_http_request except ssl.SSLCertVerificationError as e: AttributeError: [req-4zrzwrze0lrdo3bm3ovh] module 'ssl' has no attribute 'SSLCertVerificationError' During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/brett/arvados/sdk/python/tests/test_api.py", line 411, in test_initial_retry_logs api_client('v1', 'https://test.invalid/', 'NoToken', num_retries=1) File "/home/brett/arvados/sdk/python/arvados/api.py", line 280, in api_client **kwargs, File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/googleapiclient/_helpers.py", line 130, in positional_wrapper return wrapped(*args, **kwargs) File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/googleapiclient/discovery.py", line 296, in build static_discovery=static_discovery, File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/googleapiclient/discovery.py", line 422, in _retrieve_discovery_doc resp, content = req.execute(num_retries=num_retries) File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/googleapiclient/_helpers.py", line 130, in positional_wrapper return wrapped(*args, **kwargs) File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/googleapiclient/http.py", line 932, in execute headers=self.headers, File "/home/brett/arvados/sdk/python/arvados/api.py", line 73, in _retry_request response, body = _orig_retry_request(http, num_retries, *args, **kwargs) File "/home/brett/.cache/arvados-test/VENV3DIR/lib/python3.6/site-packages/googleapiclient/http.py", line 191, in _retry_request resp, content = http.request(uri, method, *args, **kwargs) File "/home/brett/arvados/sdk/python/arvados/api.py", line 118, in _intercept_http_request raise type(e)(*e.args) AttributeError: [req-4zrzwrze0lrdo3bm3ovh] module 'ssl' has no attribute 'SSLCertVerificationError' ====================================================================== ERROR: test_argument_path_unreadable_0 (tests.test_cmd_util.JSONArgumentTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/brett/arvados/sdk/python/.eggs/parameterized-0.9.0-py3.6.egg/parameterized/parameterized.py", line 620, in standalone_func return func(*(a + p.args), **p.kwargs, **kw) File "/home/brett/arvados/sdk/python/tests/test_cmd_util.py", line 165, in test_argument_path_unreadable ctx = contextlib.nullcontext AttributeError: module 'contextlib' has no attribute 'nullcontext'
Updated by Peter Amstutz 6 months ago
I'm very confused by the 'mmap.mmap' error because there's already a check for it? Somehow that doesn't work?
if self.content and hasattr(mmap.mmap, 'madvise'): self.content.madvise(mmap.MADV_WILLNEED)
Updated by Brett Smith 6 months ago
Peter Amstutz wrote in #note-8:
I'm very confused by the 'mmap.mmap' error because there's already a check for it? Somehow that doesn't work?
Bad mocking. In the tests, mmap.mmap
is patched with a mock object that returns a real mmap.mmap
object as a side effect. So hasattr(mmap.mmap, 'madvise')
returns True
because it's checking a mock object, but self.content.madvise()
runs on the real thing and fails.
Updated by Peter Amstutz 6 months ago
Brett Smith wrote in #note-9:
Bad mocking. In the tests,
mmap.mmap
is patched with a mock object that returns a realmmap.mmap
object as a side effect. Sohasattr(mmap.mmap, 'madvise')
returnsTrue
because it's checking a mock object, butself.content.madvise()
runs on the real thing and fails.
Right. Oops.
Stack overflow suggests deleting the attribute:
https://stackoverflow.com/a/65797400/17583450
So that would mean adding del mockmmap.madvise
Updated by Brett Smith 6 months ago
- Target version set to Development 2024-07-03 sprint
- Status changed from New to In Progress
21934-python36-compat @ f5a249e4cd57340ef51276460e2fb4ac41c12556
Note this is built on the 2.7-release branch. It would be fine to merge this against main too. Some of them are clear wins, others are a little more trade-off, but they should all be fine.
- All agreed upon points are implemented / addressed.
- See test notes below
- Anything not implemented (discovered or discussed during work) has a follow-up story.
- None so far
- Code is tested and passing, both automated and manual, what manual testing was done is described
- No automated testing because I'm not sure what Jenkins job is both correct and safe to run. With a Debian 10 container running Python 3.6.15 built from source, I was able to reproduce the original problem. After this branch, trying to
arv-copy
a workflow gets to the point where it fails trying to run Docker, which is gonna be non-trivial to get going in this environment. I also confirmed I could runarv-mount
by hand and browse data. After this branch, all tests are passing forsdk/python
,sdk/cwl
, andservices/fuse
.
- No automated testing because I'm not sure what Jenkins job is both correct and safe to run. With a Debian 10 container running Python 3.6.15 built from source, I was able to reproduce the original problem. After this branch, trying to
- Documentation has been updated.
- N/A
- Behaves appropriately at the intended scale (describe intended scale).
- No change in scale
- Considered backwards and forwards compatibility issues between client and server.
- Improves backwards compatibility
- Follows our coding standards and GUI style guidelines. **
Updated by Peter Amstutz 6 months ago
Brett Smith wrote in #note-11:
21934-python36-compat @ f5a249e4cd57340ef51276460e2fb4ac41c12556
Note this is built on the 2.7-release branch. It would be fine to merge this against main too. Some of them are clear wins, others are a little more trade-off, but they should all be fine.
- All agreed upon points are implemented / addressed.
- See test notes below
- Anything not implemented (discovered or discussed during work) has a follow-up story.
- None so far
- Code is tested and passing, both automated and manual, what manual testing was done is described
- No automated testing because I'm not sure what Jenkins job is both correct and safe to run. With a Debian 10 container running Python 3.6.15 built from source, I was able to reproduce the original problem. After this branch, trying to
arv-copy
a workflow gets to the point where it fails trying to run Docker, which is gonna be non-trivial to get going in this environment. I also confirmed I could runarv-mount
by hand and browse data. After this branch, all tests are passing forsdk/python
,sdk/cwl
, andservices/fuse
.- Documentation has been updated.
- N/A
- Behaves appropriately at the intended scale (describe intended scale).
- No change in scale
- Considered backwards and forwards compatibility issues between client and server.
- Improves backwards compatibility
- Follows our coding standards and GUI style guidelines. **
This LGTM.
Go ahead and merge to main.
I have to decide if we want a 2.7.3.1 release of just the Python packages, or do a 2.7.4.
Updated by Brett Smith 6 months ago
Peter Amstutz wrote in #note-12:
Go ahead and merge to main.
I had to rebase on main to avoid pulling in all the other divergent commits. Since I was doing that anyway, I skipped the commit that avoids contextlib.nullcontext
, since that one had basically no benefit for Python 3.7 on. The pushed branch is still based on 2.7-release and would be best to merge there.
I have to decide if we want a 2.7.3.1 release of just the Python packages, or do a 2.7.4.
Assigning this ticket to you to figure out the 2.7 merge and that question.
Updated by Brett Smith 6 months ago
- Assigned To changed from Brett Smith to Peter Amstutz
Updated by Peter Amstutz 6 months ago
- Status changed from In Progress to Resolved
We'll do a 2.7.4 release next week, this has been added to the 2.7-staging branch, so I am marking it as resolved.