-
Notifications
You must be signed in to change notification settings - Fork 0
Expand ASGI adapter lifecycle coverage: close on failed startup, tests, and docs #10
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
base: dev/alpha
Are you sure you want to change the base?
Changes from 1 commit
f54d7b9
6e5ced7
2d7b83f
992a520
345dc1f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,8 +69,12 @@ fallback is intentional. | |
| - On `lifespan.shutdown.complete`, the adapter awaits `bus.close()` when the owned | ||
| bus exposes a synchronous or asynchronous close method. | ||
| - Cleanup completes before shutdown success is forwarded to the ASGI server. | ||
| - Startup failures do not claim that shutdown cleanup occurred; applications should | ||
| manage resources created outside the adapter in their own lifespan handler. | ||
| - A failed startup (either `lifespan.startup.failed` or an exception escaping the | ||
| lifespan application) also closes an adapter-owned bus. Caller-owned buses remain | ||
| untouched in every failure path. | ||
| - Cleanup is idempotent: repeated startup/shutdown cycles against the same middleware | ||
| instance do not close a bus more than once. Create a new application/middleware | ||
| instance to obtain a fresh adapter-owned bus after shutdown. | ||
|
|
||
| The local `EventBus` has no resources to close. The generic close behavior exists | ||
| for application bus subclasses that coordinate transports, stores, or plugins. | ||
|
|
@@ -81,3 +85,18 @@ The bus itself is application-scoped. Request state isolates access paths, not b | |
| registrations. Use separate application instances or a custom `bus_factory` when | ||
| tests or tenants require distinct registration state. `state_key=` supports | ||
| coexistence with another request-state convention. | ||
|
|
||
| Each middleware instance creates its own bus, so concurrently running application | ||
| instances are isolated. This is process-local isolation only: pre-fork and | ||
| multi-worker deployments create one adapter-owned bus per worker. Eventful does not | ||
| coordinate registrations or delivery between workers; inject a caller-managed | ||
| transport-backed bus when cross-process behavior is required. Do not share one | ||
| adapter-owned middleware instance between event loops. | ||
|
|
||
| ## Supported versions | ||
|
|
||
| The adapters support FastAPI 0.110 or newer and Starlette 0.37 or newer on Eventful's | ||
| supported Python versions. The compatibility floor is exercised through the public | ||
| ASGI and dependency APIs; CI's normal dependency resolution also exercises current | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The compatibility-floor claim is not backed by the checked CI configuration: Useful? React with 👍 / 👎. |
||
| framework releases. Major-version compatibility is not promised until separately | ||
| validated. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,7 @@ def __init__( | |
| self._close_lock = asyncio.Lock() | ||
|
|
||
| async def __call__(self, scope: dict[str, Any], receive: Any, send: Any) -> None: | ||
| """Attach state and wait for application shutdown before closing resources.""" | ||
| """Attach state and close owned resources when a lifespan terminates.""" | ||
| if scope["type"] in {"http", "websocket"}: | ||
| scope.setdefault("state", {})[self.state_key] = self.bus | ||
|
|
||
|
|
@@ -59,12 +59,21 @@ async def __call__(self, scope: dict[str, Any], receive: Any, send: Any) -> None | |
| return | ||
|
|
||
| async def lifespan_send(message: dict[str, Any]) -> None: | ||
| """Close owned resources before reporting successful shutdown.""" | ||
| if message["type"] == "lifespan.shutdown.complete": | ||
| """Close resources before reporting shutdown or failed startup.""" | ||
| if message["type"] in { | ||
| "lifespan.startup.failed", | ||
| "lifespan.shutdown.complete", | ||
| }: | ||
| await self.close() | ||
| await send(message) | ||
|
|
||
| await self.app(scope, receive, lifespan_send) | ||
| try: | ||
| await self.app(scope, receive, lifespan_send) | ||
| except BaseException: | ||
| # A lifespan exception may prevent the application from sending either | ||
| # terminal message. Do not strand a bus that this adapter created. | ||
| await self.close() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the wrapped lifespan application raises and an adapter-owned bus's Useful? React with 👍 / 👎. |
||
| raise | ||
|
|
||
| async def close(self) -> None: | ||
| """Close an owned/opted-in bus once when it exposes `close()`.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This guarantee is false when the same supported caller-owned
bus=is injected into two middleware instances: both applications attach the identical bus and therefore share registrations and delivery state. That can mislead tests or tenants into relying on application-instance isolation that does not exist; qualify the statement as applying only to default- or factory-created buses and explicitly note that injected-bus isolation is the caller's responsibility.Useful? React with 👍 / 👎.