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

Run clients with ephemeral applications on the global event loop thread #9870

Closed
wants to merge 8 commits into from

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented Jun 8, 2023

When a client is hooked to an ephemeral application, its moved to the global event loop instead of the event loop it is created on. We do this by sending the underlying httpx client's context manager to the global event loop and patch the client.request method to send coroutines to the other thread. This is a good stepping stone towards providing a singleton client that runs on the global event loop as well as towards providing a synchronous client.

Enabling this only for the ephemeral application is motivated by #9855 which requires SQLAlchemy to be managed on a separate thread than user code.

@zanieb zanieb requested a review from a team as a code owner June 8, 2023 19:51
@netlify
Copy link

netlify bot commented Jun 8, 2023

Deploy Preview for prefect-docs-preview ready!

Name Link
🔨 Latest commit e07f406
🔍 Latest deploy log https://app.netlify.com/sites/prefect-docs-preview/deploys/64835f46162e2a0007b2c27c
😎 Deploy Preview https://deploy-preview-9870--prefect-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@zanieb
Copy link
Contributor Author

zanieb commented Jun 12, 2023

This is:

  1. A possible way to fix issues in Run tasks on the main thread #9855
  2. A path to a singleton client with a synchronous interface Allow Prefect client to be used from synchronous contexts #9695

This pull request appears to:

  1. Introduce a large performance penalty in tests; this would likely not be seen by users but we are spawning a lot of extra threads in tests since we use a lot of ephemeral client instances.
  2. Cause intermittent deadlocks

It's unclear what the best path forward is, but a more holistic approach will need to be taken.

@jakekaplan is best suited to take over this work.

@github-actions
Copy link
Contributor

This pull request is stale because it has been open 60 days with no activity. To keep this pull request open remove stale label or comment.

@zanieb
Copy link
Contributor Author

zanieb commented Aug 22, 2023

Closing this as I am not working on it and a new pull request will need to be opened if someone else takes it up.

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.

2 participants