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 > On Dec 2, 2022, at 03:39, Carlton Gibson <carlton.gib...@gmail.com> wrote: > > > 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 > > On Mon, 28 Nov 2022 at 16:53, Jon Janzen <j...@jonjanzen.com > <mailto:j...@jonjanzen.com>> wrote: >> 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 >> >> -- >> 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+unsubscr...@googlegroups.com >> <mailto:django-developers+unsubscr...@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 >> >> <https://groups.google.com/d/msgid/django-developers/CAP1wFiCp_y%3DVygQxmat-JVR2cr_LjGTKDyQ7rXtg3ERrzdfXXw%40mail.gmail.com?utm_medium=email&utm_source=footer>. > > > -- > 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+unsubscr...@googlegroups.com > <mailto:django-developers+unsubscr...@googlegroups.com>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/CAJwKpyRp5OLrEB%3DRD4zONCsChZMcBSEpqyiSCmoR9XB83Y6w5w%40mail.gmail.com > > <https://groups.google.com/d/msgid/django-developers/CAJwKpyRp5OLrEB%3DRD4zONCsChZMcBSEpqyiSCmoR9XB83Y6w5w%40mail.gmail.com?utm_medium=email&utm_source=footer>. -- 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+unsubscr...@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.
