Skip to content

Fix async life cycle management to prevent lost references to tasks. - #157 - #158

Open
sebastiaan-la-fleur wants to merge 9 commits into
mainfrom
fix_task_lifecycle
Open

sebastiaan-la-fleur wants to merge 9 commits into
mainfrom
fix_task_lifecycle

Conversation

@sebastiaan-la-fleur

@sebastiaan-la-fleur sebastiaan-la-fleur commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

PR does 2 things:

  • Fix CI
  • Fix lifecycle management for async tasks in the AsyncConnection class so task references do not get lost due to exceptions and cancellations.

@sebastiaan-la-fleur sebastiaan-la-fleur self-assigned this Sep 23, 2026
@sebastiaan-la-fleur
sebastiaan-la-fleur marked this pull request as ready for review September 23, 2026 11:46
@@ -0,0 +1,82 @@
name: Check Python

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sorry for also fixing CI in this PR, but we were running into some annoying issues that prevented the actual fix from succeeding in CI.

Copilot stopped reviewing on behalf of sebastiaan-la-fleur due to an error September 23, 2026 13:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review effort: Lite
Findings: 4 Medium severity

Open (4)
What changed in this PR

Updates the async connection’s task-lifecycle handling and expands the project’s quality gates (lint/typecheck/CI) while adding targeted unit tests to validate cancellation/stop/exception behavior.

Changes:

  • Refactors S2AsyncConnection.run() and send_msg_and_await_reception_status() to more reliably cancel/drain background tasks and surface/log exceptions.
  • Adds new async connection lifecycle unit tests and tweaks existing unit-test formatting.
  • Expands tooling: runs pyright in typecheck, adjusts lint configuration for tests, and refactors GitHub Actions CI into a reusable workflow.
File Description
tox.ini Adjusts lint/typecheck commands; adds PYTHONPATH for lint; runs pyright in tox typecheck env.
tests/​unit/​reception_status_awaiter_test.py Reformats list comprehensions for readability.
tests/​unit/​connection/​async_/​connection_test.py Adds new unit tests covering async connection task management and cancellation/stop scenarios.
src/​s2python/​connection/​async_/​connection.py Refactors background task creation/cancellation/draining and exception logging.
pyrightconfig.json Adds a tests execution environment and relaxes a specific diagnostic for tests.
pyproject.toml Raises minimum setuptools version.
mypy.ini Disables additional mypy error codes for a test-related section.
dev-requirements.txt Regenerates dev requirements (Python 3.9) and adds backports needed for older runtimes.
ci/​typecheck.sh Aggregates mypy + pyright exit status so both run and report failures.
ci/​test_unit.sh Allows passing through extra pytest args.
ci/​setup_dev_environment.sh Switches venv creation to Python 3.9.
ci/​lint.sh Splits linting for src/examples vs tests with different pylint settings.
AGENTS.md Adds contributor/development guidelines and tooling conventions.
.pylintrc Keeps certain checks enabled globally; relies on entry points to disable for tests.
.github/​workflows/​ci.yml Replaces per-job definitions with a reusable workflow and updates publish jobs.
.github/​workflows/​check-python.yml New reusable workflow to build/test/lint/typecheck per Python version.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread ci/test_unit.sh
Comment thread tests/unit/connection/async_/connection_test.py
Comment thread tox.ini
Comment thread tox.ini

This branch has not been deployed

No deployments
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