Async wrappers in contrib packages

[Jon Janzen]

Hey everyone,

Sorry if I'm not following correct protocol on this or if this has already been discussed elsewhere, but is there any consensus about (or needed for) creating async versions of contrib packages?

My personal interest in this is about django.contrib.auth (login, authenticate, etc.) and django.contrib.syndication (views.Feed) but I would guess this sort of work would fall under a general policy.

I read DEP009 and didn't see any discussion of this topic, nor could I find any discussions on the ticket tracker (my skills using the tracker are limited). I could only find 2 files mentioning "async" in django/contrib/ (git grep "async" django/contrib | grep "\.py:"):

* django/contrib/contenttypes/fields.py (added by me recently, a bug fix)

* django/contrib/staticfiles/handlers.py (added as part of standing up async support)

Has there been any consensus about this? If I'm interested in async versions of functions/features in contrib packages should I just file tickets, or is this something that might need a DEP first?

Again, sorry if I'm barking up the wrong tree. It's not my intention to waste anyone's time!

[Carlton Gibson]

Hi Jon. 

There's: https://code.djangoproject.com/ticket/31949 "Allow builtin view decorators to be applied directly to async views."

I think this is likely the next step.

There's a PR for that, which I think took a too complex approach (see discussion). A simpler (more inline) take be good to see. 

> My personal interest in this is about django.contrib.auth (login, authenticate, etc.) 

This was missing from the PR on #31949, so if you wanted to pick it up... 😜

(Not sure about the value of rewriting the built-in views though if that's what you're thinking of 🤔)

> ...and django.contrib.syndication (views.Feed) 

Not sure what you have in mind here. Perhaps explaining that in more detail would help? 

(I'm not immediately sure I see the benefit of async for the feed views? 🤔)

Kind Regards,

Carlton

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/70e80f26-e1fa-4aa9-a3a8-0d7dc38752c3n%40googlegroups.com.

[Jon Janzen]

Hey Carlton,

There's: https://code.djangoproject.com/ticket/31949 "Allow builtin view decorators to be applied directly to async views."

I think this is likely the next step.

There's a PR for that, which I think took a too complex approach (see discussion). A simpler (more inline) take be good to see. 

Thanks, I saw this ticket but it didn't look relevant when I was skimming the tracker. I'll take a closer look.

> My personal interest in this is about django.contrib.auth (login, authenticate, etc.) 

This was missing from the PR on #31949, so if you wanted to pick it up... 😜

I'll take a closer look, I think I might be able to do that :D

 

(Not sure about the value of rewriting the built-in views though if that's what you're thinking of 🤔)

> ...and django.contrib.syndication (views.Feed) 

Not sure what you have in mind here. Perhaps explaining that in more detail would help? 

(I'm not immediately sure I see the benefit of async for the feed views? 🤔)

Not for the view itself, but for individual fields that compose the Feed.

I want to define an async "item_categories" method when I subclass Feed due to an async-only permissions system I have that is out-of-scope here, but that isn't possible right now so I pre-compute each of these values and pass in a composite object with the source object and item_categories precomputed.

I would rather just declare an async function and let the framework figure out how to resolve the values reasonably efficiently for me. I don't want to pay the cost of async_to_sync for each item in the feed :/

I'm fine with setting this one aside, as I said I already have a workaround. But I can file a ticket just to track this one?

 - Jon

[Carlton Gibson]

> But I can file a ticket just to track this one?

TBH I'd prefer it if you pondered the design here without opening a ticket until such a point (if ever) that you have a concrete plan. (We'd likely just close it as wontfix unless there's a specific idea on the table anyway, so it's just noise at that point.) 

I hope that makes sense. 

Kind Regards,

Carlton

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAP1wFiCp_y%3DVygQxmat-JVR2cr_LjGTKDyQ7rXtg3ERrzdfXXw%40mail.gmail.com.

[Jon Janzen]

Hey,

Sorry about the delay in my response, holidays came early and stayed late for me this year.

TBH I'd prefer it if you pondered the design here without opening a ticket until such a point (if ever) that you have a concrete plan. (We'd likely just close it as wontfix unless there's a specific idea on the table anyway, so it's just noise at that point.) 

Understood, sorry for my ignorance on the process. I appreciate your patience!

There's: https://code.djangoproject.com/ticket/31949 "Allow builtin view decorators to be applied directly to async views."

I think this is likely the next step.

There's a PR for that, which I think took a too complex approach (see discussion). A simpler (more inline) take be good to see. 

Thanks, I saw this ticket but it didn't look relevant when I was skimming the tracker. I'll take a closer look.

Replying to myself here, I took a look at this ticket and associated PRs and that’s not quite do what I’m looking for (even if all the various constituent parts got merged) but the changes to the `auth` decorators are related.

I’m interested in an async interface of the auth app itself, i.e. the functionality exposed in `django/contrib/auth/__init__.py`.

(the next few paragraphs are background info on my personal investment in this, feel free to skip to the section marked “Proposal")

For some background on my interest in this: I’m running Django as an asgi app and all codepaths down to the django framework boundary are async. That means all my middleware, views, and ORM usage are all using the async versions, where applicable/possible. There are a number of reasons for this, but the most notable are simplicity (easier to reason about code if it is all either sync or async) and efficiency (most of the code is GraphQL resolvers which are more efficient to execute concurrently).

Also important to know that I almost exclusively use django as an API server, there is only one template for all “views” which just loads a javascript webpack bundle and renders using React, which then fetches data from the server using GraphQL. Effectively, that means I‘m calling the django auth APIs directly instead of using the default `LoginView` or `login_required` or anything like that.

Anyway, right now almost all of the sync/async boundaries are “invisible” to me in that they are inside Django. I’ve replaced all my own wrappers around the ORM’s synchronous-only methods with the `a`-prefixed methods provided over the last few Django releases (and that will be provided in 4.2!). So `aget` instead of `get`, `acreate` instead of `create` and so forth.

One area of async/sync boundaries that is currently prevalent in my codebase is my `sync_to_async` wrappers around `django.contrib.auth` methods. I’d like to push those down into Django, and then as deep into Django as is expedient right now.

# Proposal

Add asynchronous versions of the auth app’s API (i.e. `__init__.py`), then allow backends to have async-native versions and use that from the public API, and finally update the provided middleware (and maybe decorators) to use async-native functionality if they are running in ASGI mode.

## Part 1: Async API

The “simple” part of this proposal is just to define a bunch of functions with “a” prefixes in `__init__.py` and use `sync_to_async` to call the synchronous versions as has been done to, for example, QuerySet. This would involve asynchronous versions of the following functions in `auth/__init__.py`:

* `get_user`

* `authenticate`

* `login`

* `update_session_auth_hash`

* `logout`

## Part 2: async backends

I think once that interface is defined it makes sense to try and make as much of this framework as natively async-compatible as possible. That would mean adding support for async auth backends and then connecting the async API methods (as enumerated above) to the async versions of the backend methods.

This could mean that there would be no `sync_to_async` calls within the `auth` app itself (except for sessions and signals, see Open Questions below), and any sync/async boundaries would be “below” the `auth` app. For example, for `ModelBackend` and its children the sync/async boundary would be in the ORM layer, as `QuerySet` presents an async API that is currently just a wrapper around the sync internals.

Allowing async backends would involve introducing the following async functions to `BaseBackend` that just call the sync versions by default, and can be overridden in child classes to have natively async versions:

* `authenticate`

* `get_user`

* `get_user_permissions`

* `get_group_permissions`

* `get_all_permissions`

* `has_perm`

Then `ModelBackend` would be augmented to have async-native versions of the above. `RemoteUserBackend` would also need some tweaks to be async-native, but overall this isn’t terribly much work. Oh and to support `ModelBackend` being async-native `BaseUserModel` would need an async version of `get_by_natural_key`, but the implementation there is trivial.

## Part 3: async middleware

Once we have an async-native interface and backend support, I think it is natural to move the async boundary “upwards" to the middlewares. There are 2 middleware classes that currently have synthesized async versions of their implementation that could be switched to async-native:

* `AuthenticationMiddleware`

* `RemoteUserMiddleware`

These could be updated to be async-native, though the actual implementation here is a bit tricky and requires resolution of this ticket: https://code.djangoproject.com/ticket/31920

I thin this is currently out-of-scope for my proposal due to the unresolved ticket/PR there, but I think it warrants consideration so we don’t write ourselves into a corner.

## Naive Implementation

I’ve taken the time to rough-out a rather simple implementation of the above plan: https://github.com/bigfootjon/django/pull/1 (note this is a PR from my fork _to_ my fork, not a PR against the main django GitHub repo)

This branch just exists to illustrate this proposal, not as code ready for checkin. It’s missing tests and docs and… a well-considered implementation. There’s duplicated code all over the place and I didn’t even bother running the code to see if it works. I just wanted to get a feel for how this would work and though I might as well publish my hacked-up PoC to show my work.

## Open Questions

Based on my stab at the above PoC implementation I came away with several questions:

1. Are there proven strategies for reducing code duplication between sync and async versions of functionality in Django or in Python broadly? I’m not aware of any guidance here but I’m eager for resources to consider. The implementation is verbose (doubles file size in some cases) and fragile (what if a bug fix is only applied to the sync version and not the async version?) right now.

2. The auth app obviously fires several signals, and Django signals currently has a sync-only interface. I see there is an issue on the bug tracker (https://code.djangoproject.com/ticket/32172) but it is currently blocked for performance reasons. Is it fine to just add more sync_to_async wrappers here? (I did so in the above PoC implementation). Another idea is to just add a async wrapper around the `send` method and defer the rest of that ticket until the perf question can be resolved.

3. The sessions app seems to only have a synchronous interface, which would cause a lot of friction during Phase 2. I couldn’t find any tickets covering this topic (just some tangential discussion in https://groups.google.com/u/1/g/django-developers/c/D2Cm25yFzdI/m/bo_Ae_kgBQAJ), perhaps it would be a good idea for me to dig into this area first and think about asyncifying `sessions` instead of sprinkling lots of `sync_to_async` calls around `sessions` callsites in `auth`? I’d appreciate some other opinions/guidance here.

4. I’ve intentionally not considered the auth decorators as that seems like an orthogonal issue while https://code.djangoproject.com/ticket/31949 is pending, am I missing something here? Do I need to consider decorators in this proposal?

## Summary

I think without guidance my gut tells me that doing just Phase 1 would be “fine” and mirrors a lot of other work around the django codebase to add async _interfaces_ without pushing those down to natively-async _implementations_ (yet). Each phase in my proposal seems severable to me, so stopping after Phase 1 seems acceptable.

I think after Phase 1 the next step would be analyzing and adding an async interface (and, hopefully, implementation) for `sessions` before turning to Phase 2/3.

I’m well aware I dug really deep into a rabbit hole without asking if I was digging in the right spot, so please let me know where I’ve gone wrong in my analysis/approach/etiquette. I’m not invested in this plan, just in the core idea of asyncifying more of django. If the idea itself is not acceptable then I’ll be a little disappointed but I can understand the need to minimize complexity in a project like Django, so I won’t be that disappointed.

Thanks for reading this novella,

Jon

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAJwKpyRp5OLrEB%3DRD4zONCsChZMcBSEpqyiSCmoR9XB83Y6w5w%40mail.gmail.com.

[Carlton Gibson]

Hi Jon. 

Thanks for this. 

I think your use-case is reasonable, and that you're basically on the right track. 

If you were to add test cases to your PoC, there's certainly a case for looking seriously at it. 

It should be reasonable to keep pushing the interfaces down one layer at a time. (See comment on Sessions below.) 

I'm not sure a priori how Phase 2 plays out in advance. It's likely something we have to think about as we go. 

Targeted posts to the Async category of the Forum might help drive discussion. 

https://forum.djangoproject.com/c/users/async-channels/23

Let me inline some quick thoughts on your …

## Open Questions

Based on my stab at the above PoC implementation I came away with several questions:

1. Are there proven strategies for reducing code duplication between sync and async versions of functionality in Django or in Python broadly? I’m not aware of any guidance here but I’m eager for resources to consider. The implementation is verbose (doubles file size in some cases) and fragile (what if a bug fix is only applied to the sync version and not the async version?) right now.

Yes… you end up writing it twice... 🤔 Maybe in some ideal end state we end up with an async core with a thin sync wrapper around that, but we are a long way from there: I'm not sure it bears thinking about.

Part of the task on the decorators work is to try not to duplicate everything. e.g. a process_request only middleware doesn't necessarily need to consume the `get_response()` callable, so often times can make use of `markcoroutinefunction()` rather than needed to duplicate the whole middleware function. (And so on.) — But those patterns aren't 100% clear just yet — we'll find them as we go through in this next phase. 

2. The auth app obviously fires several signals, and Django signals currently has a sync-only interface. I see there is an issue on the bug tracker (https://code.djangoproject.com/ticket/32172) but it is currently blocked for performance reasons. Is it fine to just add more sync_to_async wrappers here? (I did so in the above PoC implementation). Another idea is to just add a async wrapper around the `send` method and defer the rest of that ticket until the perf question can be resolved.

So, yes... — my waiting-for-time-to-try-it-idea™ there is to sort signals into two groups on registration and call any async ones in a group concurrently, with only the single switch into the concurrent context. **Something** along those lines should be possible... 

3. The sessions app seems to only have a synchronous interface, which would cause a lot of friction during Phase 2. I couldn’t find any tickets covering this topic (just some tangential discussion in https://groups.google.com/u/1/g/django-developers/c/D2Cm25yFzdI/m/bo_Ae_kgBQAJ), perhaps it would be a good idea for me to dig into this area first and think about asyncifying `sessions` instead of sprinkling lots of `sync_to_async` calls around `sessions` callsites in `auth`? I’d appreciate some other opinions/guidance here.

So, rinse and repeat no? — We should be able to the session backend API an async interface, and so on. 

Is there a best order here, or can we just chip away bit-by-bit? 🤔

4. I’ve intentionally not considered the auth decorators as that seems like an orthogonal issue while https://code.djangoproject.com/ticket/31949 is pending, am I missing something here? Do I need to consider decorators in this proposal?

I don't think they're dependent. 

Kind Regards,

Carlton

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/6CF0F34A-A116-481A-A0E3-6EB0E2BBA302%40jonjanzen.com.

[Jon Janzen]

Hey Carlton, 

Thanks for your thoughtful comments, a few things come to mind:

A. It sounds like we’re in agreement about the utility and severability of Phase 1 (just creating an async_to_sync-based wrapper around the auth interface). I want to make sure I don’t cause extra work on the bug tracker, so does this sound like it’s ready to be filed as a ticket there? I can file the ticket and get started on writing tests/docs if so!

B. In reply to your "Is there a best order here, or can we just chip away bit-by-bit? 🤔” comment, I think there is a reasonable order: we mirror the dependencies between django internals. By that I mean I think we should asyncify the necessary parts of sessions or signals before asyncifying the parts of auth that depend on them. That reduces the need to insert “temporary” sync/async boundaries and reduces churn.

C. Based on your responses and my own thoughts in (2) I think it’s reasonable to revise the overall order into this:

1. In any order: asyncify the auth API (Phase 1), asyncify signals (ticket 32172), and asyncify sessions (TODO: discussion/ticket)

2. After all 3 parts to (1), asyncify auth backends and internals of the auth API (Phase 2)

3. Resolve ticket 31920

4. Asyncify the auth middleware (Phase 3)

D. I’m admittedly very new to Django and contributing code to it, but Phase 1 is certainly within my area of expertise considering the 2 PRs that I have contributed 😉:

https://github.com/django/django/pull/16256

https://github.com/django/django/pull/16252

After that, I’d be interested in helping nudge this along over time. I’d like to build my experience beyond just adding async wrappers 😛, so maybe I can pick up https://github.com/django/django/pull/13651 and hack away at your idea about categorizing during registration. If that all goes well I can start a discussion around asyncifying sessions (which, on first glance seems like a LARGE project of its own)

Thanks,

Jon

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAJwKpyREQT-9tHaeyFMSB5dDiwienjK6r-1y1TgnpdME8gRXPg%40mail.gmail.com.

[Carlton Gibson]

No plan ever survives first contact with the enemy :) but, this seems reasonable to me Jon, yes. 

Can I suggest we move further discussion on details to the Forum's async category? It's nicer, better contained, and we more likely to catch interested eyes. 

Thanks for pursuing this! 🎁

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/3D848ECE-47E1-4C2D-A1B6-3A3F9008635F%40jonjanzen.com.

[Jon Janzen]

Hey Carlton,

Sorry I skipped over the suggestion in one of your earlier messages to post on the Forum.

I’ve done so now: https://forum.djangoproject.com/t/asyncifying-django-contrib-auth-and-signals-and-maybe-sessions/18770

I think we can consider this thread deprecated in favor of the above Forum thread.

Cheers,

Jon

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAJwKpyQuU9%3DUwgOfbpd4u1b2W%3DNeQ%3DYwv4S5kJVodERPgwggPg%40mail.gmail.com.

[Carlton Gibson]

Great. Thanks Jon. I will follow up there next week. 

Hey Carlton,

Jon

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAP1wFiCp_y%3DVygQxmat-JVR2cr_LjGTKDyQ7rXtg3ERrzdfXXw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAJwKpyRp5OLrEB%3DRD4zONCsChZMcBSEpqyiSCmoR9XB83Y6w5w%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/6CF0F34A-A116-481A-A0E3-6EB0E2BBA302%40jonjanzen.com.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAJwKpyREQT-9tHaeyFMSB5dDiwienjK6r-1y1TgnpdME8gRXPg%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/3D848ECE-47E1-4C2D-A1B6-3A3F9008635F%40jonjanzen.com.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAJwKpyQuU9%3DUwgOfbpd4u1b2W%3DNeQ%3DYwv4S5kJVodERPgwggPg%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/1B076D08-3F13-402E-9951-60AC3F108290%40jonjanzen.com.