-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Response cleanup #2586
Comments
I don't think there should be a callback API on @app.get("/")
async def handler(request: Request):
response = await request.respond()
await response.send("foobar")
await response.eof() # trigger sending of response and enable pipelining to accept another request here
await cleanup() Perhaps with function other than There would be much use for such functionality, including that of writing something to database or performing other operations that may take time after the response has already been sent, in many cases specific to one handler only and possibly requiring access to resources (such as |
This was the original thought and is arguably a nicer api for all the reasons you mentioned. But it would be a huge breaking change since we allow the task to be Cancelled early when the client disconnects (which is what it would do with an explicit |
What about making it a setter style decorator? @handler.cleanup
async def handler_cleanup(...): |
Still cannot apply within I guess that implementing it the way I suggested could be somewhat more complicated for Sanic's own implementation, but it also would make a lot more sense from app perspective. Other async frameworks such as NodeJS Express already function like this, and do not care when the handler returns. Going for a decorator that way also means that the module where the cleanup is (possibly some utility module) needs to import the handler. The other way around (using cleanup decorator on the handler function) might be better and it preserves the same import direction as your original proposition. However, defining it inside the function as originally proposed at least allows passing variables as arguments, and having conditional execution if set in The complication in implementing it the way I proposed would come that either the resumption of the pipeline or the remainder of the handler would need to be executed as a separate task, both of which are troublesome to implement, at least. Granted, Express gets this easier, with Javascript not having to await for function calls to return but can simply abandon them (as global tasks, if you will). |
Before posting this, I spent about 5 minutes looking to see what it would take to defer task cancellation when a client closes. I had abandoned the thought because it was certainly much more complex. But, potentially we can differentiate when you complete the response inside the handler and when it happens outside (ie, |
I have no take on that. Only that sending the response inside handler was supposed to be first class API, and after-handler handling only a fallback that calls it (even if in practice most handlers would return a response, not send it). |
You are correct. Scrap the shortcut and let's fix this for real. @app.get("/")
async def handler(request: Request):
response = await request.respond()
await response.send("foobar")
await response.eof()
# Client is free to close connection or start another request
await do_something_here()
|
It would be nice to have a simple API to direct post-response cleanup efforts outside of the task. Consider the following example:
In this example, we would immediately schedule a task using
add_task
API to allow the response to return as early as possible. But, it should also be smart enough to work even withouteof()
being called such that it would wait for the response to be sent and cleaned before proceeding to cleanup. Then you could even have handlers like this and guarantee thatcleanup
is run on a completed response.The text was updated successfully, but these errors were encountered: