fix(BA-1976): Make AsyncEtcd an explicit async context manager#5289
fix(BA-1976): Make AsyncEtcd an explicit async context manager#5289HyeockJinKim merged 12 commits intomainfrom
Conversation
1a10632 to
2d8c9ea
Compare
9552aac to
4af738e
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the etcd-client-py dependency from version 0.4.1 to 0.5.1 and refactors the AsyncEtcd wrapper to be an explicit async context manager. The changes leverage improvements in the etcd-client-py library that ensure graceful tokio runtime shutdown, eliminating the need for manual workarounds.
Changes:
- Updated etcd-client-py from 0.4.1 to 0.5.1 and testcontainers from 4.8.1 to 4.13.3
- Made AsyncEtcd and LegacyEtcdLoader explicit async context managers with aenter and aexit methods
- Refactored all AsyncEtcd usage sites to use async with context managers instead of manual close() calls
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai/backend/common/etcd.py | Added async context manager protocol to AsyncEtcd with aenter and aexit methods |
| src/ai/backend/manager/config/loader/legacy_etcd_loader.py | Added async context manager protocol to LegacyEtcdLoader |
| src/ai/backend/storage/server.py | Updated etcd_ctx to use async with instead of manual close() |
| src/ai/backend/storage/dependencies/infrastructure/etcd.py | Updated provide method to use async with pattern |
| src/ai/backend/manager/server.py | Refactored etcd_ctx to initialize etcd within async with |
| src/ai/backend/manager/dependencies/bootstrap/etcd.py | Updated provide method to use async with pattern |
| src/ai/backend/manager/cli/context.py | Refactored etcd_ctx, config_ctx, and redis_ctx to use async with patterns |
| src/ai/backend/install/context.py | Updated etcd_ctx to use async with instead of manual close() |
| src/ai/backend/agent/server.py | Updated etcd_ctx to use async with instead of manual close() |
| src/ai/backend/agent/dependencies/bootstrap/etcd.py | Updated provide method to use async with pattern |
| src/ai/backend/cli/main.py | Removed workaround sleep for tokio/pyo3-async-runtimes shutdown race |
| tests/conftest.py | Updated test fixtures to use async with pattern |
| tests/unit/common/conftest.py | Updated gateway_etcd fixture to use async with |
| tests/unit/common/test_distributed.py | Updated test to use async with for AsyncEtcd |
| tests/unit/common/health_checker/checkers/test_etcd.py | Updated test fixtures to use async with |
| tests/unit/storage-proxy/conftest.py | Changed mock_etcd to async fixture using async with |
| tests/unit/storage/dependencies/infrastructure/test_redis.py | Updated test fixture to use async with |
| requirements.txt | Updated etcd-client-py and testcontainers versions |
| python.lock | Updated lock file with new dependency versions |
| changes/5289.fix.md | Added changelog entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| exc_type: type[BaseException], | ||
| exc_val: BaseException, | ||
| exc_tb: TracebackType, |
There was a problem hiding this comment.
The exc_type, exc_val, and exc_tb parameters should be Optional types to match Python's async context manager protocol. The correct signature should be exc_type: Optional[type[BaseException]], exc_val: Optional[BaseException], exc_tb: Optional[TracebackType]. This ensures the type hints match the protocol when aexit is called without an exception (None values).
| exc_type: type[BaseException], | |
| exc_val: BaseException, | |
| exc_tb: TracebackType, | |
| exc_type: Optional[type[BaseException]], | |
| exc_val: Optional[BaseException], | |
| exc_tb: Optional[TracebackType], |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Confirmed successful installation in our internal dev-test farm, which had issues without |
resolves #xxx (BA-1976)
Now we can leverage:
Checklist: (if applicable)