♻️ Adding lifespan support for FastAPI & migrated dynamic-scheduler to use it

[GitHK]

What do these changes do?

While working on #7070 it became necessary to have a "setup" function that can use a context manger. Unfortunately FastAPI does not allow us to mix the old setup functions and the new lifespan.

What does this bring:

• ✨ utility to combine multiple lifespans into a single one which guarantees the expected order of setup and teardown
• ♻️ dynamic-scheduler is the first service which was totally ported

New complications

Previously each module had its own setup function to clearly state that something required initialisation. Since events were being used for setup and teardown, there was no clear need to distinguish the 3 phases of the FastAPI setup:

• before lifecycle
• lifecycle startup
• lifecycle shutdown

As viewed below.

# CURRENT USAGE

def setup(app: FastAPI) -> None:
    # lifecycle before
    async def startup()-> None:
        # lifecycle startup
    async def shutdown()-> None:
        # lifecycle shutdown

    app.add_event_handler("startup", startup)
    app.add_event_handler("shutdown", shutdown)


app = FastAPI()
# rest of setup
setup(app)

# run the app at some point

With the lifespan the before lifecycle part cannot be included in the lifespan.
Some modules like the prometheus_instrumentation had to be split for this reason.

# NEW USAGE
from fastapi_lifespan_manager import State
from servicelib.fastapi.lifespan_utils import combine_lifespans

def initialize(app: FastAPI) -> None:
    # lifecycle before

async def lifespan(app: FastAPI) -> AsyncIterator[State]:
    # lifecycle startup
    yield
    # lifecycle shutdown


app = FastAPI(lifespan=combine_lifespans(lifespan))
# rest of setup
initialize(app)

# run the app at some point

Coding style extension

Prefixes for functions used to configure the module during the setting up phase:

• setup can be used as before including before, startup and shutdown
• lifespan can only include startup and shutdown
• intialize only includes before

Maybe initialize (as a name) can be improved but I would still keep this structure to avoid confusion when poring over the other services.

Related issue/s

• required by ✨ adding docker-api-proxy service ⚠️ #7070

How to test

Dev-ops checklist

• No ENV changes or I properly updated ENV (read the instruction)

[codecov]

Codecov Report

Attention: Patch coverage is 78.21229% with 39 lines in your changes missing coverage. Please review.

Project coverage is 87.58%. Comparing base (4352166) to head (8758e10).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7149      +/-   ##
==========================================
- Coverage   87.72%   87.58%   -0.14%     
==========================================
  Files        1635     1277     -358     
  Lines       63943    54006    -9937     
  Branches     1178      651     -527     
==========================================
- Hits        56093    47302    -8791     
+ Misses       7538     6524    -1014     
+ Partials      312      180     -132     

Flag	Coverage Δ
integrationtests	64.70% <50.00%> (-0.10%)	⬇️
unittests	85.55% <78.21%> (-0.56%)	⬇️

Components	Coverage Δ
api	∅ <ø> (∅)
pkg_aws_library	∅ <ø> (∅)
pkg_dask_task_models_library	∅ <ø> (∅)
pkg_models_library	∅ <ø> (∅)
pkg_notifications_library	∅ <ø> (∅)
pkg_postgres_database	∅ <ø> (∅)
pkg_service_integration	∅ <ø> (∅)
pkg_service_library	74.09% <26.53%> (-0.31%)	⬇️
pkg_settings_library	∅ <ø> (∅)
pkg_simcore_sdk	85.50% <ø> (ø)
agent	96.46% <ø> (ø)
api_server	90.55% <50.00%> (ø)
autoscaling	96.08% <ø> (ø)
catalog	90.33% <100.00%> (ø)
clusters_keeper	99.24% <ø> (ø)
dask_sidecar	91.26% <ø> (ø)
datcore_adapter	93.19% <ø> (ø)
director	76.92% <ø> (-0.09%)	⬇️
director_v2	91.29% <50.00%> (-0.15%)	⬇️
dynamic_scheduler	97.17% <99.19%> (-0.04%)	⬇️
dynamic_sidecar	89.75% <ø> (ø)
efs_guardian	90.25% <ø> (ø)
invitations	93.28% <ø> (ø)
osparc_gateway_server	∅ <ø> (∅)
payments	92.66% <ø> (ø)
resource_usage_tracker	89.06% <ø> (+0.10%)	⬆️
storage	89.57% <ø> (ø)
webclient	∅ <ø> (∅)
webserver	86.23% <ø> (ø)

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4352166...8758e10. Read the comment docs.

[sanderegg]

Hmm I like the approach to change this to use the lifespan but I am not convinced how this goes now.

I would prefer that you inspire yourself from how aiohttp did it:

• no need to create an asynccontextmanager everytime, it is just a standard python Generator such as

# db.py
  async def _setup_db(app: FastAPI) -> AsyncGenerator[None]:
    # do the init stuff
    yield
    # do the cleanup stuff

get_lifespan_manager(app).add(_setup_db)

[GitHK]

Hmm I like the approach to change this to use the lifespan but I am not convinced how this goes now.

I would prefer that you inspire yourself from how aiohttp did it:

• no need to create an asynccontextmanager everytime, it is just a standard python Generator such as

# db.py
  async def _setup_db(app: FastAPI) -> AsyncGenerator[None]:
    # do the init stuff
    yield
    # do the cleanup stuff

get_lifespan_manager(app).add(_setup_db)

I am not in favour of this. Since a the concept of a context manger is exactly what you like. It runs some code "initially", "yields" execution and "finally" runs code to cleanup.
When using a generator, you can yield the execution for as many times as you want. From my point of view a context manager looks more similar to what we are tying to achieve a "setup"/ "teardown" pattern.

Anybody has more opinions on this? @pcrespov @matusdrobuliak66 @giancarloromeo

[pcrespov]

Hmm I like the approach to change this to use the lifespan but I am not convinced how this goes now.
I would prefer that you inspire yourself from how aiohttp did it:

• no need to create an asynccontextmanager everytime, it is just a standard python Generator such as

# db.py
  async def _setup_db(app: FastAPI) -> AsyncGenerator[None]:
    # do the init stuff
    yield
    # do the cleanup stuff

get_lifespan_manager(app).add(_setup_db)

I am not in favour of this. Since a the concept of a context manger is exactly what you like. It runs some code "initially", "yields" execution and "finally" runs code to cleanup. When using a generator, you can yield the execution for as many times as you want. From my point of view a context manager looks more similar to what we are tying to achieve a "setup"/ "teardown" pattern.

Anybody has more opinions on this? @pcrespov @matusdrobuliak66 @giancarloromeo

@GitHK I think @sanderegg proposal does not contradict yours. What he is basically suggesting is to expose that functionality as a decorator, which is perfectly possible in this case.

I like the ideas you are proposing here. IMO it just need a bit of tuning (I will prepare some examples in my review)
To get you a preview, what @sanderegg and you are suggesting is already implemented in https://github.com/uriyyo/fastapi-lifespan-manager

[GitHK]

Hmm I like the approach to change this to use the lifespan but I am not convinced how this goes now.
I would prefer that you inspire yourself from how aiohttp did it:

• no need to create an asynccontextmanager everytime, it is just a standard python Generator such as

# db.py
  async def _setup_db(app: FastAPI) -> AsyncGenerator[None]:
    # do the init stuff
    yield
    # do the cleanup stuff

get_lifespan_manager(app).add(_setup_db)

I am not in favour of this. Since a the concept of a context manger is exactly what you like. It runs some code "initially", "yields" execution and "finally" runs code to cleanup. When using a generator, you can yield the execution for as many times as you want. From my point of view a context manager looks more similar to what we are tying to achieve a "setup"/ "teardown" pattern.
Anybody has more opinions on this? @pcrespov @matusdrobuliak66 @giancarloromeo

@GitHK I think @sanderegg proposal does not contradict yours. What he is basically suggesting is to expose that functionality as a decorator, which is perfectly possible in this case.

I like the ideas you are proposing here. IMO it just need a bit of tuning (I will prepare some examples in my review) To get you a preview, what @sanderegg and you are suggesting is already implemented in https://github.com/uriyyo/fastapi-lifespan-manager

OK tried with the suggested library.
Only issue is that the State does not work as expected. Even if it's yield, till all the states handlers finished the setup, no state will be saved.

[pcrespov]

thx. left some suggestions

[pcrespov]

THOUGHT: ideally i would enforce this comment with code (e.g. with a decorator that if the app is started, it raises)

[GitHK]

I would not know how to do that.

I think this is also fine since an error will be raised pointing you to this function.

[pcrespov]

What about checking that the state that you initialize in this initializer is not initialized? :-)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud