PermissionRequiredMixin not importable before apps ready?
Hi devs, I have a weird problem that I first thought is on my side. But after thinking about it, it could be fixed in Django with a no-brainer and backwards-compatible. But before filing a bug, I'd like to ask here for your opinion - it may be that I am stil wrong. I use a custom view mixin class that is used for some views in my application, and it is also an interface for a plugin system, so it will be loaded at **declaration time** (before apps & models are loaded). All good so far, but nearly all of my views need django's PermissionRequiredMixin. So I decided to add PermissionRequiredMixin directly to my parent mixin class (which gets loaded early). Neither my class nor PermissionRequiredMixin uses models, but just by loading `django.contrib.auth.mixins` there is one line that hickups Django: from django.contrib.auth.viewsimport redirect_to_login This leads to importing "User", hence a model, and Django throws a AppRegistryNotReady exception. In django.contrib.auth.mixins, the only method that uses "redirect_to_login()" is line 60 in handle_no_permission(). This "problem" could be easily solved if the import statement is done locally in handle_no_permission, and not at module level - which leads to the django.contrib.auth.mixins being usable in early Django (before model loading). The only side effect I could think of, that it adds some loading time during that method call the first time - but reduces the same loading time at startup. Which is neglegible IMHO. Please tell me if I'm completely wrong - or if this is worth a bug report/PR. Christian -- Dr. Christian González https://nerdocs.at -- 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/87640740-b78e-5ecf-ba2a-ef53c1ba10a7%40nerdocs.at.
Re: Async wrappers in contrib packages
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 On Mon, 6 Feb 2023 at 03:22, Jon Janzen wrote: > 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 inte
Re: PermissionRequiredMixin not importable before apps ready?
Hello Christian, Could you possibly provide a minimal project setup that reproduces your issue? >From what I can see here you're running into this issue because the `django.contrib.auth.views` module calls `get_user_model` at initialization time but the latter has been passing `require_ready=False` to `apps.get_model` for a while[0] so it's hard to tell whats to blame without a proper traceback. I suspect you might have an import cycle between involving your custom user model and some of your dependencies on django.contrib.auth. Cheers, Simon [0] https://github.com/django/django/commit/cb7bbf97a74fa7800865e3615f196ad65dc4f281 Le mardi 7 février 2023 à 09:14:22 UTC-5, Christian González a écrit : > Hi devs, > > I have a weird problem that I first thought is on my side. But after > thinking about it, it could be fixed in Django with a no-brainer and > backwards-compatible. But before filing a bug, I'd like to ask here for > your opinion - it may be that I am stil wrong. > > I use a custom view mixin class that is used for some views in my > application, and it is also an interface for a plugin system, so it will be > loaded at **declaration time** (before apps & models are loaded). > > All good so far, but nearly all of my views need django's > PermissionRequiredMixin. So I decided to add PermissionRequiredMixin > directly to my parent mixin class (which gets loaded early). Neither my > class nor PermissionRequiredMixin uses models, but just by loading > `django.contrib.auth.mixins` there is one line that hickups Django: > > from django.contrib.auth.views import redirect_to_login > > This leads to importing "User", hence a model, and Django throws a > AppRegistryNotReady exception. > > In django.contrib.auth.mixins, the only method that uses > "redirect_to_login()" is line 60 in handle_no_permission(). > > This "problem" could be easily solved if the import statement is done > locally in handle_no_permission, and not at module level - which leads to > the django.contrib.auth.mixins being usable in early Django (before model > loading). > > The only side effect I could think of, that it adds some loading time > during that method call the first time - but reduces the same loading time > at startup. Which is neglegible IMHO. > > Please tell me if I'm completely wrong - or if this is worth a bug > report/PR. > > Christian > > -- > Dr. Christian Gonzálezhttps://nerdocs.at > > -- 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/71b01d85-06fb-49f5-8a1f-d34a9024e991n%40googlegroups.com.
