Skip to content
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

Support generator function to manage startup/shutdown. #799

Merged
merged 6 commits into from May 4, 2020

Conversation

tomchristie
Copy link
Member

@tomchristie tomchristie commented Jan 14, 2020

An alternate implementation of #785

  • Refactor lifespan to run as an async generator
  • Allow lifespan_context=... argument to Starlette and Router.
  • Add app argument to the lifespan context function.

End result API would be allowing the application to take a single async generator function, that can encapsulate any required startup/shutdown logic.

Taking the example in #785, you'd end up with this kind of pattern...

async def lifespan(app):
    async with db.ConnectionPool() as app.state.pool:
        yield

app = Starlette(routes=..., middleware=..., lifespan=lifespan)

@tomchristie tomchristie changed the title Refactor lifespan to run as an async generator Support lifespan async generator to Starlette(..., lifespan=...) Jan 14, 2020
@tomchristie tomchristie changed the title Support lifespan async generator to Starlette(..., lifespan=...) Support async generator function to manage startup/shutdown. Jan 14, 2020
@sm-Fifteen
Copy link

Using this approach, I have two questions:

  1. Couldn't an application use a sync generator instead of an async one as their lifespan context? After all, servers are forbidden from accepting connections until the a startup complete event is recieved, so there isn't a whole lot of things that could preempt the startup coroutines regardless of whether the functions being called are blocknig or not. There might not be any async code to run on startup or shutdown.
  2. Could this be a step in fixing lifespan events in sub-applications #649?

@tomchristie
Copy link
Member Author

  1. The async function is sufficient, since it can run either sync or async code. It might be graceful to be able to have it accept either kind of function, or it might be unnecessary extra noise?
  2. Potentially, yeah.

@sm-Fifteen
Copy link

1. The async function is sufficient, since it can run either sync or async code. It _might_ be graceful to be able to have it accept either kind of function, or it might be unnecessary extra noise?

Having looked into it, you can't loop though a sync and an async iterator with the same kind of loop, so it would mean duplicating the iteration part, which would be unneccessarily noisy. It probably wouldn't be worth it anyway, so nervermind that.

2. Potentially, yeah.

That's good if it turns out that way, lifespan has a number of caveats that make it unreliable (#649) or impractical (what this PR attempts to fix) to use right now. That's probably out of scope for this PR, though.

@iksteen
Copy link

iksteen commented Jan 21, 2020

Works for me, just be sure to properly document that lifespan and startup/shutdown are mutually exclusive (at least, in this implementation).

@tomchristie tomchristie changed the title Support async generator function to manage startup/shutdown. Support generator function to manage startup/shutdown. Jan 29, 2020
@tomchristie
Copy link
Member Author

Couldn't an application use a sync generator instead of an async one as their lifespan context?

Alrighty, for completeness I've added support for the function to be either a generator or an async generator.

@rugleb
Copy link

rugleb commented Mar 6, 2020

When can we expect a release with this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants