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

replace individual calls to asyncio.run with global event loop #94

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pmeier
Copy link
Contributor

@pmeier pmeier commented Feb 2, 2024

This Pull request fixes #93

  • I completed the issue numbers (#xx) in the sentence above. The word "fixes" should remain in front of each issue
  • My PR is tagged as draft when I'm still working on it, and I remove the draft flag when it is ready for review.
  • I added one or several changelog.md entries in the appropriate "in progress" section (not the last release one)

b - My PR fixes some issues:

(Delete this section if not relevant)

  • Each issue is well-described, well-labeled, and contains reproducible code examples.

@smarie This PR needs cleanup, but I wanted to get early feedback from you if the overall functionality is what you want.

I think it would make sense to move all the async stuff to a dedicated module or put them into utils. Any opinion here?

@smarie
Copy link
Owner

smarie commented Mar 8, 2024

Thanks @pmeier for this nice contribution.
The issue description thread looks good and the proposed solution as well.

I think that it would be a good idea, if not too difficult, to have a specific test in your test file reproducing the issue #93 explicitly, so that it can ensure non-regression in the future.

Indeed some utils_asyncio.py module could be a good idea, if enough material is to go there. But don't rush into this, you can also keep using utils.py if the amount of code for async is not too much. I let you decide what is most convenient for you.

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.

asyncio functionality is mostly unusable after async handling was applied
2 participants