Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev-python/python-language-server: Add Python 3.8 compatibility #16732

Closed

Conversation

mattst88
Copy link
Contributor

All tests pass for both dev-python/python-language-server and dev-python/python-jsonrpc-server under Python 3.8.

While testing, I noticed that test/plugins/test_completion.py::test_pyqt_completion will fail if dev-python/PyQt5 is built without USE=widgets.

______________________________________________________________________________________________ test_pyqt_completion _______________________________________________________________________________________________

config = <pyls.config.config.Config object at 0x7fe8af47ab90>, workspace = <pyls.workspace.Workspace object at 0x7fe8b06e5f90>

    @pytest.mark.skipif(PY2 or (sys.platform.startswith('linux') and os.environ.get('CI') is not None),
                        reason="Test in Python 3 and not on CIs on Linux because wheels don't work on them.")
    def test_pyqt_completion(config, workspace):
        # Over 'QA' in 'from PyQt5.QtWidgets import QApplication'
        doc_pyqt = "from PyQt5.QtWidgets import QA"
        com_position = {'line': 0, 'character': len(doc_pyqt)}
        doc = Document(DOC_URI, workspace, doc_pyqt)
        completions = pyls_jedi_completions(config, doc, com_position)
    
>       assert completions is not None
E       assert None is not None

test/plugins/test_completion.py:149: AssertionError

which I guess makes sense because the test tries to import from PyQt5.QtWidgets. I'm not sure if we want to add an explicit test dependency. I suppose so?

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @mattst88
Areas affected: ebuilds
Packages affected: dev-python/python-jsonrpc-server, dev-python/python-language-server

dev-python/python-jsonrpc-server: @AndrewAmmerlaan, @gentoo/proxy-maint
dev-python/python-language-server: @AndrewAmmerlaan, @gentoo/proxy-maint

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.

If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels Jul 17, 2020
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-07-17 21:41 UTC
Newest commit scanned: 2cfe922
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/dc42186cbe/output.html

@AndrewAmmerlaan
Copy link
Member

Hmm, interesting, I remember I had a failure somewhere when I tried, will try again.

@mattst88
Copy link
Contributor Author

Should I hold this PR until you've had a chance to investigate the test failure I mentioned? In case I was unclear, it's an existing failure that happens with Python 3.7.

@AndrewAmmerlaan
Copy link
Member

AndrewAmmerlaan commented Jul 19, 2020

I get:

=============================================================================================== FAILURES ================================================================================================
__________________________________________________________________________________________ test_request_error ___________________________________________________________________________________________

endpoint = <pyls_jsonrpc.endpoint.Endpoint object at 0x7f7d5cfdb8b0>, consumer = <MagicMock id='140176407771360'>

def test_request_error(endpoint, consumer):
future = endpoint.request('methodName', {'key': 'value'})
assert not future.done()

consumer.assert_called_once_with({
'jsonrpc': '2.0',
'id': MSG_ID,
'method': 'methodName',
'params': {'key': 'value'}
})

# Send an error back from the client
error = exceptions.JsonRpcInvalidRequest(data=1234)
>       endpoint.consume({
'jsonrpc': '2.0',
'id': MSG_ID,
'error': error.to_dict()
})

test/test_endpoint.py:86: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pyls_jsonrpc/endpoint.py:109: in consume
self._handle_response(message['id'], message.get('result'), message.get('error'))
pyls_jsonrpc/endpoint.py:241: in _handle_response
request_future.set_result(result)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <Future at 0x7f7d5d012a30 state=finished raised JsonRpcInvalidRequest>, result = None

def set_result(self, result):
"""Sets the return value of work associated with the future.

Should only be used by Executor implementations and unit tests.
"""
with self._condition:
if self._state in {CANCELLED, CANCELLED_AND_NOTIFIED, FINISHED}:
>               raise InvalidStateError('{}: {!r}'.format(self._state, self))
E               concurrent.futures._base.InvalidStateError: FINISHED: <Future at 0x7f7d5d012a30 state=finished raised JsonRpcInvalidRequest>

/usr/lib/python3.8/concurrent/futures/_base.py:524: InvalidStateError
__________________________________________________________________________________________ test_request_cancel __________________________________________________________________________________________

endpoint = <pyls_jsonrpc.endpoint.Endpoint object at 0x7f7d5cf71310>, consumer = <MagicMock id='140176407335264'>

def test_request_cancel(endpoint, consumer):
future = endpoint.request('methodName', {'key': 'value'})
assert not future.done()

consumer.assert_called_once_with({
'jsonrpc': '2.0',
'id': MSG_ID,
'method': 'methodName',
'params': {'key': 'value'}
})

# Cancel the request
future.cancel()
consumer.assert_any_call({
'jsonrpc': '2.0',
'method': '$/cancelRequest',
'params': {'id': MSG_ID}
})

with pytest.raises(exceptions.JsonRpcException) as exc_info:
>           assert future.result(timeout=2)

test/test_endpoint.py:119: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <Future at 0x7f7d5cfdb640 state=cancelled>, timeout = 2

def result(self, timeout=None):
"""Return the result of the call that the future represents.

Args:
timeout: The number of seconds to wait for the result if the future
isn't done. If None, then there is no limit on the wait time.

Returns:
The result of the call that the future represents.

Raises:
CancelledError: If the future was cancelled.
TimeoutError: If the future didn't finish executing before the given
timeout.
Exception: If the call raised then that exception will be raised.
"""
with self._condition:
if self._state in [CANCELLED, CANCELLED_AND_NOTIFIED]:
>               raise CancelledError()
E               concurrent.futures._base.CancelledError

/usr/lib/python3.8/concurrent/futures/_base.py:430: CancelledError
------------------------------------------------------------------------------------------- Captured log call -------------------------------------------------------------------------------------------
ERROR    concurrent.futures:_base.py:330 exception calling callback for <Future at 0x7f7d5cfdb640 state=cancelled>
Traceback (most recent call last):
File "/usr/lib/python3.8/concurrent/futures/_base.py", line 328, in _invoke_callbacks
callback(self)
File "/var/tmp/portage/dev-python/python-jsonrpc-server-0.3.4/work/python-jsonrpc-server-0.3.4/pyls_jsonrpc/endpoint.py", line 91, in callback
future.set_exception(JsonRpcRequestCancelled())
File "/usr/lib/python3.8/concurrent/futures/_base.py", line 539, in set_exception
raise InvalidStateError('{}: {!r}'.format(self._state, self))
concurrent.futures._base.InvalidStateError: CANCELLED: <Future at 0x7f7d5cfdb640 state=cancelled>
=========================================================================================== warnings summary ============================================================================================
test/test_endpoint.py::test_bad_message
/var/tmp/portage/dev-python/python-jsonrpc-server-0.3.4/work/python-jsonrpc-server-0.3.4/pyls_jsonrpc/endpoint.py:101: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
log.warn("Unknown message type %s", message)

test/test_endpoint.py::test_consume_notification_method_not_found
/var/tmp/portage/dev-python/python-jsonrpc-server-0.3.4/work/python-jsonrpc-server-0.3.4/pyls_jsonrpc/endpoint.py:138: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
log.warn("Ignoring notification for unknown method %s", method)

test/test_endpoint.py::test_consume_request_cancel_unknown
/var/tmp/portage/dev-python/python-jsonrpc-server-0.3.4/work/python-jsonrpc-server-0.3.4/pyls_jsonrpc/endpoint.py:168: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
log.warn("Received cancel notification for unknown message id %s", msg_id)

-- Docs: https://docs.pytest.org/en/latest/warnings.html
======================================================================================== short test summary info ========================================================================================
FAILED test/test_endpoint.py::test_request_error - concurrent.futures._base.InvalidStateError: FINISHED: <Future at 0x7f7d5d012a30 state=finished raised JsonRpcInvalidRequest>
FAILED test/test_endpoint.py::test_request_cancel - concurrent.futures._base.CancelledError
=============================================================================== 2 failed, 25 passed, 3 warnings in 0.14s ================================================================================
* ERROR: dev-python/python-jsonrpc-server-0.3.4::localrepo failed (test phase):
*   Tests fail with python3.8

on python-jsonrpc-server.

Could you show me your depgraph, so we can compare what is different?

Here's mine:

* Searching for python-jsonrpc-server in dev-python ...

* dependency graph for dev-python/python-jsonrpc-server-0.3.4
`--  dev-python/python-jsonrpc-server-0.3.4  ~amd64 
`--  dev-python/mock-3.0.5-r1  (dev-python/mock) amd64  [python_targets_python3_7(-)? python_targets_python3_8(-)? -python_single_target_python3_7(-) -python_single_target_python3_8(-)]
`--  dev-python/pycodestyle-2.6.0  (dev-python/pycodestyle) ~amd64  [python_targets_python3_7(-)? python_targets_python3_8(-)? -python_single_target_python3_7(-) -python_single_target_python3_8(-)]
`--  dev-python/pyflakes-2.2.0  (dev-python/pyflakes) ~amd64  [python_targets_python3_7(-)? python_targets_python3_8(-)? -python_single_target_python3_7(-) -python_single_target_python3_8(-)]
`--  dev-python/pylint-2.5.3  (dev-python/pylint) ~amd64  [python_targets_python3_7(-)? python_targets_python3_8(-)? -python_single_target_python3_7(-) -python_single_target_python3_8(-)]
`--  dev-python/ujson-1.35-r1  (~dev-python/ujson-1.35) ~amd64  [python_targets_python3_7(-)? python_targets_python3_8(-)? -python_single_target_python3_7(-) -python_single_target_python3_8(-)]
`--  dev-lang/python-3.7.8-r1  (dev-lang/python) amd64
`--  dev-lang/python-3.8.4  (dev-lang/python) ~amd64
`--  dev-lang/python-exec-2.4.6-r1  (>=dev-lang/python-exec-2) amd64  [python_targets_python3_7(-)? python_targets_python3_8(-)? -python_single_target_python3_7(-) -python_single_target_python3_8(-)]
`--  dev-python/versioneer-0.18-r1  (dev-python/versioneer) amd64  [python_targets_python3_7(-)? python_targets_python3_8(-)? -python_single_target_python3_7(-) -python_single_target_python3_8(-)]
`--  dev-python/pytest-5.4.3  (>=dev-python/pytest-4.5.0) ~amd64  [python_targets_python3_7(-)? python_targets_python3_8(-)? -python_single_target_python3_7(-) -python_single_target_python3_8(-)]
`--  dev-python/setuptools-46.4.0-r1  (>=dev-python/setuptools-42.0.2) ~amd64  [python_targets_python3_7(-)? python_targets_python3_8(-)? -python_single_target_python3_7(-) -python_single_target_python3_8(-)]
[ dev-python/python-jsonrpc-server-0.3.4 stats: packages (12), max depth (1) ]

In case I was unclear, it's an existing failure that happens with Python 3.7.

Instead of depending on PyQt5 directly, I would prefer to expand the QtPy dependency to include the gui flag, e.g.:

dev-python/QtPy[gui,testlib,${PYTHON_USEDEP}]

instead of

dev-python/QtPy[testlib,${PYTHON_USEDEP}]

That should pull in PyQt5 with both the gui and the widgets flag, as the QtPy ebuild has:

gui? ( dev-python/PyQt5[${PYTHON_USEDEP},gui,widgets] )

I would prefer this solution, because QtPy is an abstraction layer for PyQt5 and PySide2, and in principle this package should work with both (even though PySide2 is not packaged in ::gentoo), therefore I would like to avoid a direct dependency on PyQt5.

@mattst88
Copy link
Contributor Author

 % equery depgraph python-jsonrpc-server
 * Searching for python-jsonrpc-server ...

 * dependency graph for dev-python/python-jsonrpc-server-0.3.4
 `--  dev-python/python-jsonrpc-server-0.3.4  ~amd64 
   `--  dev-python/mock-3.0.5-r1  (dev-python/mock) amd64  [python_targets_python3_7(-)? -python_single_target_python3_7(-)]
   `--  dev-python/pycodestyle-2.6.0  (dev-python/pycodestyle) amd64  [python_targets_python3_7(-)? -python_single_target_python3_7(-)]
   `--  dev-python/pyflakes-2.2.0  (dev-python/pyflakes) amd64  [python_targets_python3_7(-)? -python_single_target_python3_7(-)]
   `--  dev-python/pylint-2.4.4-r1  (dev-python/pylint) amd64  [python_targets_python3_7(-)? -python_single_target_python3_7(-)]
   `--  dev-python/ujson-1.35-r1  (~dev-python/ujson-1.35) ~amd64  [python_targets_python3_7(-)? -python_single_target_python3_7(-)]
   `--  dev-lang/python-3.7.8-r2  (dev-lang/python) amd64 
   `--  dev-lang/python-exec-2.4.6-r1  (>=dev-lang/python-exec-2) amd64  [python_targets_python3_7(-)? -python_single_target_python3_7(-)]
   `--  dev-python/versioneer-0.18-r1  (dev-python/versioneer) amd64  [python_targets_python3_7(-)? -python_single_target_python3_7(-)]
   `--  dev-python/pytest-5.4.3  (>=dev-python/pytest-4.5.0) amd64  [python_targets_python3_7(-)? -python_single_target_python3_7(-)]
   `--  dev-python/setuptools-46.4.0-r1  (>=dev-python/setuptools-42.0.2) amd64  [python_targets_python3_7(-)? -python_single_target_python3_7(-)]
[ dev-python/python-jsonrpc-server-0.3.4 stats: packages (11), max depth (1) ]

The only difference I see is that I have pylint-2.4.4-r1 and you have 2.5.3. I installed 2.5.3 and the python-jsonrpc-server tests still pass for me.

But in running the tests again, I noticed this

 * python3_8: running distutils-r1_run_phase python_test
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.7.8, pytest-5.4.3, py-1.8.0, pluggy-0.13.1 -- /usr/bin/python3.7

Says it's still using 3.7.

So I emerged dev-python/pytest with PYTHON_TARGETS="python3_7 python3_8" (it only had python3_7 before) and I now see the same failures you saw.

Crap, so I guess it's not ready.

@mattst88 mattst88 closed this Jul 20, 2020
@mattst88
Copy link
Contributor Author

Oh, but I guess this means that python-jsonrpc-server is missing a test dep on pytest. Please mention @mattst88 in your PRs for these packages and I'll be happy to merge them.

@mattst88
Copy link
Contributor Author

FWIW, looks like there's an open PR upstream at palantir/python-jsonrpc-server#37

@AndrewAmmerlaan
Copy link
Member

AndrewAmmerlaan commented Jul 20, 2020

Oh, but I guess this means that python-jsonrpc-server is missing a test dep on pytest.

The distutils_enable_tests pytest line should add a dependency on pytest AFAIK.

The distutils-r1 eclass has:

		pytest)
			test_pkg=">=dev-python/pytest-4.5.0"
			python_test() {
				pytest -vv || die "Tests fail with ${EPYTHON}"
			}
			;;

and

			test_deps+=" $(python_gen_cond_dep "
				${test_pkg}[\${PYTHON_MULTI_USEDEP}]
			")"

and

			BDEPEND+=" test? ( ${test_deps} )"

And pytest is shown in the python-jsonrpc-server dependency graph, so I'm a bit confused as to why the ebuild didn't complain about pytest not being installed with the correct PYTHON_TARGETS.

Please mention @mattst88 in your PRs for these packages and I'll be happy to merge them.

Awesome, thank you!

@mattst88
Copy link
Contributor Author

It looks like the upstream PR I linked to has been merged. Do you think we should apply those patches and enable Python 3.8?

@mattst88 mattst88 reopened this Aug 18, 2020
@AndrewAmmerlaan
Copy link
Member

It looks like the upstream PR I linked to has been merged. Do you think we should apply those patches and enable Python 3.8?

I think that is a good idea. The latest release is from January, but no functional changes seem to have been made since then, so I don't expect upstream to release a new version soon. And if we can get py3.8 here, I can probably add it to spyder-5.0 as well (#17148).

Closes: gentoo#16732
Signed-off-by: Matt Turner <mattst88@gentoo.org>
Signed-off-by: Matt Turner <mattst88@gentoo.org>
@mattst88 mattst88 force-pushed the python-language-server-python3_8 branch from 2cfe922 to 01b29a8 Compare August 19, 2020 03:53
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-08-19 04:12 UTC
Newest commit scanned: 01b29a8
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/dac5d02c27/output.html

@mattst88
Copy link
Contributor Author

(Untested so far. Hopefully tomorrow)

@AndrewAmmerlaan
Copy link
Member

(Untested so far. Hopefully tomorrow)

Just tested this, works for me 👍
Thanks

NeddySeagoon pushed a commit to NeddySeagoon/gentoo-arm64 that referenced this pull request Aug 21, 2020
Closes: gentoo#16732
Signed-off-by: Matt Turner <mattst88@gentoo.org>
@mattst88 mattst88 deleted the python-language-server-python3_8 branch July 22, 2021 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits.
Projects
None yet
4 participants