-
Notifications
You must be signed in to change notification settings - Fork 10
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
Expected behavior with BackgroundTask #23
Comments
Sorry for the late reply @adriangb but this is currently the expected, but obviously not terribly helpful, behaviour. I have made some plans which I partially explain in the now-close-to-3-year-old issue here: #15 I've got some vacation coming up, where I hope to find the time to complete the refactor to emit two timings, one for the "response", which then does not include the background task, and another for the "request" which includes the background task time. I am open to suggestions for naming these things better, but the diagram in the link above should explain rather well what I have in mind. |
Well I'm just curious if it's a wanted or unwanted feature. We could change how background tasks work in Starlette or at least 3rd party libraries, but I want to understand how this would impact things like this middleware. In other words, if background tasks ran outside of the ASGI request / response and thus it was not possible to time them with an ASGI middleware, would that be... a good thing? |
I did not expect them to be included when I wrote this, although I had every opportunity to read the source and be aware of it, it did come as a surprise. So to me it was an unwanted feature, but I obviously have not been hard pressed to do anything about it, just look at the age of the issue pointing it out 😁 If you go ahead and change it, I suppose it would be nice to have a way to either wrap their calls or hook into when they complete, as I wouldn't be surprised some users would prefer the functionality to stay the same, which is why I had plans to be able to emit timings for both time-until-response and total time of the request. |
Noted, I think it should be pretty easy to provide a telemetry hook for background tasks! To be concrete, I'm thinking about something like this: https://github.com/adriangb/asgi-background |
What is the expected behavior when dealing with background tasks? Should the time it takes to execute them be included or excluded from the timings?
The text was updated successfully, but these errors were encountered: