Add lifespan context v3 #5503

JonathanPlasse · 2022-10-16T15:19:03Z

rebased @uSpike's work from #2944 addressing #2943

codecov · 2022-10-16T15:20:30Z

Codecov Report

Patch coverage: 100.00% and no project coverage change

Comparison is base (cf73051) 100.00% compared to head (302adda) 100.00%.

❗ Current head 302adda differs from pull request most recent head 0ef7cfa. Consider uploading reports for the commit 0ef7cfa to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #5503   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          540       540           
  Lines        13969     13971    +2     
=========================================
+ Hits         13969     13971    +2

Impacted Files	Coverage Δ
fastapi/applications.py	`100.00% <ø> (ø)`
fastapi/routing.py	`100.00% <ø> (ø)`
tests/test_router_events.py	`100.00% <100.00%> (ø)`
tests/test_datastructures.py	`100.00% <0.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

github-actions · 2022-10-16T15:26:36Z

📝 Docs preview for commit 807f811 at: https://634c221ee1f2e413b23dcacc--fastapi.netlify.app

JarroVGIT

I think it's better to import specific type annotations rather than the whole of Starlette class, just for typing.

Is there a reason you call this 'lifespan context v3'? What does the v3 refer to? This is quite established behaviour in Starlette (by the looks of it; around 3 years in or so).

JarroVGIT · 2022-10-20T19:17:35Z

fastapi/routing.py

@@ -41,6 +42,7 @@
 from pydantic.error_wrappers import ErrorWrapper, ValidationError
 from pydantic.fields import ModelField, Undefined
 from starlette import routing
+from starlette.applications import Starlette


You only use this import for Typing purposes, wouldn't it make more sense to make use of the more general ASGIApp type in starlette.types?

In starlette.routing the type used is Any.

JarroVGIT · 2022-10-20T19:24:34Z

tests/test_router_events.py

+
+def test_app_lifespan_state(state: State) -> None:
+    @asynccontextmanager
+    async def lifespan(app: Starlette) -> AsyncIterator[None]:


This would be a AsynGenerator, no🤔? Because of the yield in this function.

ref in code:

fastapi/fastapi/concurrency.py

Line 19 in f67b19f

) -> AsyncGenerator[_T, None]:

According to StackOverflow, AsyncIterator is also correct as a generator is an iterator, so it is correct and also less complex to type.

Every generator is an iterator, but not every iterator is a generator. The point of typing in python is to provide information on the object type as precise as possible. If your take your argument to the extreme, everything could be typed as object and it would be technically correct, but not really useful.

I don't agree with "less complex to type", it is literally one argument more (as generators have a 'send' type and a 'yield' type)?

You are right, I made the change.

JonathanPlasse · 2022-10-21T17:57:07Z

I think it's better to import specific type annotations rather than the whole of Starlette class, just for typing.

Is there a reason you call this 'lifespan context v3'? What does the v3 refer to? This is quite established behaviour in Starlette (by the looks of it; around 3 years in or so).

This is the 3rd pull request made to add lifespan to FastAPI.

github-actions · 2022-10-21T17:59:11Z

📝 Docs preview for commit e4f1776 at: https://6352ddd0da9e0a4530022b64--fastapi.netlify.app

JarroVGIT · 2022-10-21T18:55:10Z

fastapi/routing.py

@@ -480,6 +481,7 @@ def __init__(
        route_class: Type[APIRoute] = APIRoute,
        on_startup: Optional[Sequence[Callable[[], Any]]] = None,
        on_shutdown: Optional[Sequence[Callable[[], Any]]] = None,
+        lifespan: Optional[Callable[[Any], AsyncContextManager[Any]]] = None,


Now it is just inconsistent, no? It's typed FastAPI on L74 in applications.py, but here it's typed Any, while I would still argue the correct typing would be the ASGIApp from starlette.types. Because, if you have different orders of middleware (which would all be ASGIApp) you don't have to specifically have a FastAPI app object in the lifespan param.

Interestingly this is actually how it is typed in Starlette I just noticed. But my remark would remain the same, am I missing something maybe?

Any could be replaced by FastAPi which would only be imported if typing.TYPE_CHECKING is True (during static analysis and not runtime).

github-actions · 2022-10-21T20:24:32Z

📝 Docs preview for commit 8c47d6a at: https://6352ffe1a3b96c66122559fc--fastapi.netlify.app

github-actions · 2022-10-22T08:17:48Z

📝 Docs preview for commit 1df567d at: https://6353a7059f22b0626235cbf9--fastapi.netlify.app

github-actions · 2022-10-22T08:45:50Z

📝 Docs preview for commit 302adda at: https://6353ad93c6297166c53d82dc--fastapi.netlify.app

JonathanPlasse · 2022-10-27T14:52:53Z

All requested changes have been made.

github-actions · 2022-11-15T10:37:54Z

📝 Docs preview for commit 62b8340 at: https://63736bd69dcd6b37faffe1b1--fastapi.netlify.app

github-actions · 2022-11-27T14:57:39Z

📝 Docs preview for commit c5c9b64 at: https://63837abfda05a672ca4a4c33--fastapi.netlify.app

github-actions · 2022-12-16T21:46:20Z

📝 Docs preview for commit 87f9445 at: https://639ce70189c20f052ba4cf44--fastapi.netlify.app

generator lifespans were deprectated in starlette 0.16.0 https://github.com/encode/starlette/releases/tag/0.16.0

github-actions · 2023-01-17T14:42:12Z

📝 Docs preview for commit 2c05463 at: https://63c6b375d289a546e6b326ee--fastapi.netlify.app

github-actions · 2023-03-06T17:34:34Z

📝 Docs preview for commit 0ef7cfa at: https://640623f5c1f3f70f5eff81ff--fastapi.netlify.app

tiangolo · 2023-03-07T15:47:41Z

Thank you @JonathanPlasse! 🍰

And thanks for the help @JarroVGIT, @Kludex, @yezz123 🙇

I included your commits in the original PR #2944 and merged that one.

So I'll now close this one.

This will be available in FastAPI 0.93.0, released in the next few hours. 🎉

Sorry for the long delay! 🙈 I wanted to personally address each issue/PR and they piled up through time, but now I'm checking each one in order.

yezz123 approved these changes Oct 19, 2022

View reviewed changes

JarroVGIT suggested changes Oct 20, 2022

View reviewed changes

JonathanPlasse force-pushed the add-lifespan-context-v3 branch from 807f811 to e7c761c Compare October 21, 2022 17:51

JarroVGIT suggested changes Oct 21, 2022

View reviewed changes

JonathanPlasse force-pushed the add-lifespan-context-v3 branch from 1df567d to 302adda Compare October 22, 2022 08:41

tiangolo added the investigate label Nov 7, 2022

JonathanPlasse force-pushed the add-lifespan-context-v3 branch from 302adda to 62b8340 Compare November 15, 2022 10:33

JonathanPlasse force-pushed the add-lifespan-context-v3 branch from 62b8340 to c5c9b64 Compare November 27, 2022 14:54

uSpike and others added 11 commits January 17, 2023 15:32

Add lifespan context argument

6f9ebea

Add tests for lifespan context and refactor router event tests

edef8c0

Add lifespan docs

76968ca

Fix lifespan typing

0dbcb35

Fix lifespan doc highlight

2712fb6

Fix event tests repeated code

d15df2d

Fix import sorting

9665645

update lifespan to be an async context manager

628aa71

generator lifespans were deprectated in starlette 0.16.0 https://github.com/encode/starlette/releases/tag/0.16.0

Fix formatting

f612d74

Fix type of lifespan to AsyncContextManager[Any]

f13274c

Fix docs for lifespan asynccontextmanager

0eed654

JonathanPlasse added 3 commits January 17, 2023 15:32

Use @asynccontextmanager for lifespan for test

81591ba

Use FastAPI instead of Starlette for lifespan

d60f9b8

Replace lifespan AsyncIterator by AsyncGenerator

2c05463

JonathanPlasse force-pushed the add-lifespan-context-v3 branch from 87f9445 to 2c05463 Compare January 17, 2023 14:36

Merge branch 'master' into add-lifespan-context-v3

0ef7cfa

tiangolo added a commit to uSpike/fastapi that referenced this pull request Mar 6, 2023

🔀 Merge PR tiangolo#5503 from @JonathanPlasse with updated types

8b6dd1a

tiangolo mentioned this pull request Mar 7, 2023

✨ Add support for lifespan async context managers (superseding startup and shutdown events) #2944

Merged

tiangolo closed this Mar 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add lifespan context v3 #5503

Add lifespan context v3 #5503

JonathanPlasse commented Oct 16, 2022

codecov bot commented Oct 16, 2022 •

edited

github-actions bot commented Oct 16, 2022

JarroVGIT left a comment

JarroVGIT Oct 20, 2022

JonathanPlasse Oct 21, 2022

JarroVGIT Oct 20, 2022

Kludex Oct 21, 2022

JonathanPlasse Oct 21, 2022

JarroVGIT Oct 21, 2022

JonathanPlasse Oct 21, 2022

JonathanPlasse commented Oct 21, 2022

github-actions bot commented Oct 21, 2022

JarroVGIT Oct 21, 2022

JarroVGIT Oct 21, 2022

JonathanPlasse Oct 21, 2022

github-actions bot commented Oct 21, 2022

github-actions bot commented Oct 22, 2022

github-actions bot commented Oct 22, 2022

JonathanPlasse commented Oct 27, 2022

github-actions bot commented Nov 15, 2022

github-actions bot commented Nov 27, 2022

github-actions bot commented Dec 16, 2022

github-actions bot commented Jan 17, 2023

github-actions bot commented Mar 6, 2023

tiangolo commented Mar 7, 2023

Add lifespan context v3 #5503

Add lifespan context v3 #5503

Conversation

JonathanPlasse commented Oct 16, 2022

codecov bot commented Oct 16, 2022 • edited

Codecov Report

github-actions bot commented Oct 16, 2022

JarroVGIT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JonathanPlasse commented Oct 21, 2022

github-actions bot commented Oct 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 21, 2022

github-actions bot commented Oct 22, 2022

github-actions bot commented Oct 22, 2022

JonathanPlasse commented Oct 27, 2022

github-actions bot commented Nov 15, 2022

github-actions bot commented Nov 27, 2022

github-actions bot commented Dec 16, 2022

github-actions bot commented Jan 17, 2023

github-actions bot commented Mar 6, 2023

tiangolo commented Mar 7, 2023

codecov bot commented Oct 16, 2022 •

edited