Skip to content

Conversation

@kristjanvalur
Copy link
Contributor

@kristjanvalur kristjanvalur commented Sep 2, 2025

asyncio.ensure_future is an anacronism. Since python version 3.7, create_task() is preferred over ensure_future.

Moreover, error handling is more natural (and traceback friendly) by using wrapper methods instead of
future completion callbacks.

  • We add some unit tests for the asyncSlot() exception case.
  • We trigger error handling for asyncClose()
  • We ignore CancelledError for asyncClose()
  • We add unit tests for asyncClose()

@kristjanvalur kristjanvalur marked this pull request as ready for review September 2, 2025 13:50
@github-actions
Copy link

github-actions bot commented Sep 2, 2025

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/qasync
  __init__.py
  _windows.py
  tests
  test_qeventloop.py 900
Project Total  

This report was generated by python-coverage-comment-action

@kristjanvalur kristjanvalur marked this pull request as draft September 2, 2025 14:41
@kristjanvalur kristjanvalur force-pushed the create-task branch 2 times, most recently from 49c6fe3 to 783eacc Compare September 2, 2025 18:09
@kristjanvalur kristjanvalur marked this pull request as ready for review September 3, 2025 14:12
@hosaka hosaka self-assigned this Sep 18, 2025
@hosaka hosaka merged commit 9addefc into CabbageDevelopment:master Sep 23, 2025
74 checks passed
@hosaka
Copy link
Collaborator

hosaka commented Sep 23, 2025

@kristjanvalur thank you for contributing!

@kristjanvalur
Copy link
Contributor Author

I should add that the previous implementation of asyncClose had a problem: Any uncaught exceptions in the close slot would be silently ignored, since the future's result() was never evaluated. I came across this in production recently so it is worth getting this pr released at some point.

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