Migrate to pygls v2.0a2
#928
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pygls in `v2.0a2` switched from using the low-level asyncio APIs to using the high-level one and surprisingly, this broke `esbonio`. The server would start ok, but wouldn't launch Sphinx processes, the preview command did nothing, it wouldn't even produce any log messages! That is, until the user changed a configuration setting. After changing a setting - any setting, the server suddenly springs into life, logs, previews, sphinx processes all of them would start working as if nothing was wrong. I eventually managed to figure out that the callbacks registered by the configuration system on the server's `ready` future were never being called. ``` self.server.ready.add_done_callback(self._notify_subscriptions) ``` By why would changing the asyncio API in use break these callbacks?! After spending some time with the debugger I eventually spotted the culprit ``` >>> asyncio.get_running_loop() <_UnixSelectorEventLoop running=True closed=False debug=False> >>> self.server.ready._loop <_UnixSelectorEventLoop running=False closed=False debug=False> ``` The server's `ready` future was using a different event loop and because the event loop is not running, when the future is resolved, the callbacks were never scheduled! While I cannot explain why this was not an issue before, I can explain why it is an issue now. The `ready` future is created in the constructor of the `EsbonioLanguageServer` class - before any event loop has been created and so it uses a new one based on the current event loop policy[1][2]. Unfortunately, when pygls later starts its main loop by calling `asyncio.run()` it creates a new event loop, orphaning the `ready` future in its unused event loop[3] Since we don't need to use the `ready` future asynchronously, the simplest fix is to convert it to future from the `concurrent.futures` module, removing the need for an event loop completely. [1]: https://github.com/python/cpython/blob/307c63358681d669ae39e5ecd814bded4a93443a/Lib/asyncio/futures.py#L79 [2]: https://github.com/python/cpython/blob/307c63358681d669ae39e5ecd814bded4a93443a/Lib/asyncio/events.py#L791 [3]: https://github.com/python/cpython/blob/307c63358681d669ae39e5ecd814bded4a93443a/Lib/asyncio/runners.py#L146
- Use new `pygls.io_` infrastructure - Use new `websockets.asyncio` API
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.