Fix Starlette 1.0 compatibility: convert on_startup/on_shutdown to lifespan#849
Fix Starlette 1.0 compatibility: convert on_startup/on_shutdown to lifespan#849namemartin wants to merge 2 commits intoAnswerDotAI:mainfrom
Conversation
…fespan Starlette 1.0 removed on_startup/on_shutdown params from Starlette.__init__(). Add _wrap_lifespan() to convert these callbacks into a lifespan context manager, composing with any user-provided lifespan. Fixes AnswerDotAI#847.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Found 1 changed notebook. Review the changes at https://app.gitnotebooks.com/AnswerDotAI/fasthtml/pull/849 |
There was a problem hiding this comment.
Pull request overview
Updates FastHTML’s Starlette integration to stay compatible with Starlette 1.0 by translating legacy on_startup/on_shutdown callbacks into a lifespan context manager that composes with any user-provided lifespan.
Changes:
- Add
_wrap_lifespan()helper to composeon_startup/on_shutdownwith an optional userlifespan. - Update
FastHTML.__init__to pass onlylifespan=toStarlette.__init__(removingon_startup=/on_shutdown=kwargs). - Add regression tests covering basic app creation and lifecycle ordering across sync/async handlers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
fasthtml/core.py |
Introduces _wrap_lifespan() and routes startup/shutdown through lifespan for Starlette 1.0 compatibility. |
nbs/api/00_core.ipynb |
Source notebook changes for the same lifespan wrapping logic (nbdev sync). |
tests/test_lifespan.py |
New tests validating startup/shutdown behavior and lifespan composition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fasthtml/core.py
Outdated
| for h in on_startup: await h() if inspect.iscoroutinefunction(h) else h() | ||
| if lifespan: | ||
| async with lifespan(app) as state: yield state | ||
| else: yield | ||
| for h in on_shutdown: await h() if inspect.iscoroutinefunction(h) else h() |
There was a problem hiding this comment.
Using inspect.iscoroutinefunction(h) is not sufficient to detect async work here (e.g., functools.partial(async_fn), callable instances with async __call__, or sync wrappers returning an awaitable). This can lead to un-awaited coroutine objects. Call the handler first and then await the result if it is awaitable (e.g., via inspect.isawaitable).
| for h in on_startup: await h() if inspect.iscoroutinefunction(h) else h() | |
| if lifespan: | |
| async with lifespan(app) as state: yield state | |
| else: yield | |
| for h in on_shutdown: await h() if inspect.iscoroutinefunction(h) else h() | |
| for h in on_startup: | |
| res = h() | |
| if inspect.isawaitable(res): | |
| await res | |
| if lifespan: | |
| async with lifespan(app) as state: yield state | |
| else: yield | |
| for h in on_shutdown: | |
| res = h() | |
| if inspect.isawaitable(res): | |
| await res |
There was a problem hiding this comment.
Kept iscoroutinefunction for simplicity
nbs/api/00_core.ipynb
Outdated
| " for h in on_startup: await h() if inspect.iscoroutinefunction(h) else h()\n", | ||
| " if lifespan:\n", | ||
| " async with lifespan(app) as state: yield state\n", | ||
| " else: yield\n", | ||
| " for h in on_shutdown: await h() if inspect.iscoroutinefunction(h) else h()\n", |
There was a problem hiding this comment.
inspect.iscoroutinefunction won’t catch common async handler forms like partial(async_fn) or callable objects with async __call__, and can leave coroutine results un-awaited. Consider invoking each handler and awaiting the result when it’s awaitable (e.g., inspect.isawaitable(result)).
| " for h in on_startup: await h() if inspect.iscoroutinefunction(h) else h()\n", | |
| " if lifespan:\n", | |
| " async with lifespan(app) as state: yield state\n", | |
| " else: yield\n", | |
| " for h in on_shutdown: await h() if inspect.iscoroutinefunction(h) else h()\n", | |
| " for h in on_startup:\n", | |
| " res = h()\n", | |
| " if inspect.isawaitable(res): await res\n", | |
| " if lifespan:\n", | |
| " async with lifespan(app) as state: yield state\n", | |
| " else: yield\n", | |
| " for h in on_shutdown:\n", | |
| " res = h()\n", | |
| " if inspect.isawaitable(res): await res\n", |
There was a problem hiding this comment.
Kept iscoroutinefunction for simplicity
Wrap the lifespan yield in try/finally so on_shutdown callbacks execute even when the lifespan context manager raises an exception. Add test to verify this behavior.
Related Issue
Fixes #847
Proposed Changes
Starlette 1.0 removed
on_startup/on_shutdownparams fromStarlette.__init__(). This adds_wrap_lifespan()to convert these callbacks into a lifespan context manager. Unlike #848, this composes with any user-providedlifespaninstead of silently dropping the callbacks.Types of changes
Checklist
Additional Information
Closes #848's approach by composing
on_startup/on_shutdownwithlifespanrather than prioritizing one over the other. Also usesinspect.iscoroutinefunctioninstead of the deprecatedasyncio.iscoroutinefunction.