-
Notifications
You must be signed in to change notification settings - Fork 960
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
add support for lifespan tasks #3312
Conversation
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.
The tasks should have a chance to clean up nicely without being cancelled right?
I think it's up to the task itself to handle being cancelled cleanly, by using |
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.
Looks good.
I'd like to see at least one test in the integration tests that exercises this.
Maybe have a lifespan task and lifespan contextmanager set a module global and send it to the frontend via an rx.var
Previously the SessionStorage util was just looking in localStorage, but the tests didn't catch it because they were asserting the token was not None, rather than asserting it was truthy. Fixed here, because I'm using this structure in the new lifespan test.
Fix CI failure
Add support for running task during the whole lifespan of the app, rather than linked to any specific state.