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

Fix async implementation not supported #204

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

eastonsuo
Copy link

No description provided.

@ccordoba12
Copy link
Member

ccordoba12 commented May 9, 2022

Hey @eastonsuo, thanks for your contribution! Could you explain when this is needed?

@eastonsuo
Copy link
Author

eastonsuo commented May 10, 2022

I want to use asyncio in plugins, such as when do a async http request for another service, or wait for a asyncio.Event and blocked.

(Maybe Endpoint should be changed to AsyncEndpoint to chieve that?) @ccordoba12

Hey @eastonsuo, thanks for your contribution! Could you explain when this is needed?

@ccordoba12 ccordoba12 changed the title fix async impl not supported #203 Fix async implementation not supported May 28, 2022
@@ -6,7 +6,7 @@
import os
import socketserver
import threading

import asyncio
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this to be on top of from functools import partial and leave the blank line here.

@ccordoba12
Copy link
Member

I want to use asyncio in plugins, such as when do a async http request for another service, or wait for a asyncio.Event and blocked.

I see, thanks for the explanation.

(Maybe Endpoint should be changed to AsyncEndpoint to chieve that?)

I'm not really sure. @andfoy, what do you think?

@ccordoba12 ccordoba12 requested a review from andfoy May 28, 2022 16:54
@andfoy
Copy link
Contributor

andfoy commented May 31, 2022

I'm not really sure. @andfoy, what do you think?

@eastonsuo, could you please clarify your asynchronous case of use? We would assume that you would like to get multiple future objects instead of blocking on a call?

@eastonsuo
Copy link
Author

I'm not really sure. @andfoy, what do you think?

@eastonsuo, could you please clarify your asynchronous case of use? We would assume that you would like to get multiple future objects instead of blocking on a call?

yes, for example, I would like to post a request asynchronously within a call and on the other hand not blocking another call.

I think it can be realized by an AsyncEndpoint and start server by asyncio.run()

@lieryan
Copy link
Contributor

lieryan commented Nov 24, 2022

The idea of async handler is reasonable, but pylsp already start some threads to run hook handlers that may take time (e.g. linters). It may be reasonable to extend this so that all hook handlers have the option to be written as either def or async def. Either way, IMO hook handlers should not be running on the main pylsp thread, so that one slow/broken plugins should not interfere with the performance of the rest of python-lsp-server.

However, this should not be implemented in the flatten() method as suggested in this PR. This doesn't look like the right place to implement that.

@ccordoba12
Copy link
Member

Agreed. I think the proper solution is to make the server fully async aware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants