Async compatible debug-toolbar middleware

[salty-ivy]

Description

GSoC 2024

This PR aims to add async compatibility in django-debug-toolbar middleware

Fixes # (issue)

Checklist:

• I have added the relevant tests for this change.
• I have added an item to the Pending section of docs/changes.rst.

[matthiask]

Thanks!

[matthiask]

LGTM. I wonder how the panels can differentiate in process_request between the sync and async version. Also, awaiting def process_request (NOT async def process_request) looks a bit strange, but seems fine to me after thinking about it a bit.

[salty-ivy]

LGTM. I wonder how the panels can differentiate in process_request between the sync and async version.

panels would receive either WSGIRequest or ASGIRequest ( not async ) instance which are subclasses of HttpRequest.

Also, awaiting def process_request (NOT async def process_request) looks a bit strange, but seems fine to me after thinking about it a bit.

process_request call of panels would ultimately reach down to view via chain of calls of both panels and then middlewares, the response will always be adapted to async by adapt_method_mode that has to be awaited.

https://github.com/django/django/blob/e56a32b89bb7fadffdfaa2cdf12b4863ccd5af9b/django/core/handlers/base.py#L104

it even adapts the view if its not async def the response will be adapted to async coroutines if the receiving middleware is async and we are running in async context ( if both conditions are true ).

https://github.com/django/django/blob/e56a32b89bb7fadffdfaa2cdf12b4863ccd5af9b/django/core/handlers/base.py#L54

this is my overall understanding so far.

[tim-schilling]

This is coming along nicely! Thank you for all your work. I have a few comments on particular aspects. We also need to document the new panel API before this can be merged in.

[salty-ivy]

I have changed the approach to test async panel compatibility tests.

[tim-schilling]

Code-wise, this is great! Thank you Aman. I think we need to update the docs in two areas:

1. Change Panel.is_async to a @property method with a doc string, then update https://github.com/jazzband/django-debug-toolbar/blob/main/docs/panels.rst?plain=1#L332 to include that attribute in the documentation.
2. Update the architecture document here:

   django-debug-toolbar/docs/architecture.rst

   Lines 82 to 84 in 9bcd6ca

   	- Support for async and multi-threading: This is currently unsupported, but
   	is being implemented as per the
   	`Async compatible toolbar project <https://github.com/orgs/jazzband/projects/9>`_.

[salty-ivy]

1. Change Panel.is_async to a @property method with a doc string, then update https://github.com/jazzband/django-debug-toolbar/blob/main/docs/panels.rst?plain=1#L332 to

Sorry for the nit pick question but should we use @classproperty or @property with class level private attribute _is_async? my main idea is that is should be class level we might need to access it in future directly via class and makes subclassing easy

[tim-schilling]

Not a nitpick at all. It's a good question @salty-ivy! My instinct is to go with @property to be consistent with the others. @matthiask thoughts?

[matthiask]

Autogenerating documentation from code is always a workaround for lazyness (I like it a lot!) but I'm not sure we should uglify the code just for that.

I thought that adding a #: comment above a property would also act as a docstring by the way, so that we could keep the attribute?

[tim-schilling]

TIL, then let's go with what Matthias suggested. Thank you for pointing that out

[matthiask]

If you search this page for #: you'll find examples:
https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#directive-autoproperty

So, if the autodoc support is the only reason for converting the attribute to a property I think I'm -0 on it. No strong opposition certainly, I just don't see the need for it.

[tim-schilling]

I wanted the autodoc and then thought it would bring consistency to the API. Though you and @salty-ivy both questioning it is enough for me to agree to having it as an attribute.

[salty-ivy]

2. Update the architecture document here:

   django-debug-toolbar/docs/architecture.rst

   Lines 82 to 84 in 9bcd6ca

   	- Support for async and multi-threading: This is currently unsupported, but
   	is being implemented as per the
   	`Async compatible toolbar project <https://github.com/orgs/jazzband/projects/9>`_.

So we are left with this only while keeping is_async a class level attribute for now.

[tim-schilling]

Sounds good.

[salty-ivy]

Not sure, may be something related to sphinx missing in the updated docs to throw the lint error?

[tim-schilling]

@salty-ivy it can be hard to read the docs failures:

architecture.rst:84: : Spell check: ASGI: are disabled by default when running in async/ASGI enviroment..
architecture.rst:84: : Spell check: enviroment: are disabled by default when running in async/ASGI enviroment..

You need to change enviroment to environment

[salty-ivy]

You need to change enviroment to environment

ahh, how can I miss this, thank you so much. Interestingly It even fails on ASGI keyword.

[tim-schilling]

See! I didn't even catch that other one. I thought it was two lines because the word was used twice on that line 😁

[tim-schilling]

I squashed the commits down to one, cleaned up the commit message, and rebased on main. After tests pass, I'll merge it in