On Fri, Sep 20, 2013 at 12:11 PM, Luke Sneeringer <l...@sneeringer.com>wrote:
> > > Sent from my iPad > > On Sep 19, 2013, at 9:24 PM, Russell Keith-Magee <russ...@keith-magee.com> > wrote: > > > On Fri, Sep 20, 2013 at 10:53 AM, <gavinw...@gmail.com> wrote: > >> > Note that EmailUser *doesn't* have a Meta: swappable definition. There >> is nothing on this model that says "I am swappable", or "I am an >> appropriate substitute for User". >> >> Ah, this is were the misunderstanding was. authtools does actually set >> swappable on its replacement user, too[1]. The two definitions look like >> this: >> >> class User(Model): >> username = models.CharField() >> USERNAME_FIELD = 'username' >> class Meta: >> swappable = 'AUTH_USER_MODEL' >> >> class EmailUser(Model): >> email = models.CharField() >> USERNAME_FIELD = 'email' >> class Meta: >> swappable = 'AUTH_USER_MODEL' >> >> > How does Django identify that EmailUser *shouldn't* be synchronised to >> the database? >> >> The existing swappable mechanism takes care of it. Only the model >> referenced by settings.AUTH_USER_MODEL will be installed, not both. >> > > Ah. Thats… interesting. I'm moderately surprised it works at all. I'll > need to dig in to the internals to work out *why* it works… swappable > certainly wasn't intended to work like that. There might be some > interesting land mines lying in wait. > > > Aha, I think I found at least some of my disconnect. But I tested this > yesterday, and it definitely does do that. > Agreed - this explains the source of the disconnect. > I know I just asked this in a previous email, but I will repeat for ease > of others possibly following this thread: can you provide a brief overview > of what swappable *does* do? It seems to be on observation it does exactly > this. You say it's intended to do something else. What is that something > else? > The intention was to mark a particular model as a something that can be replaced. It's about marking the endpoint that can be swapped, not predeclaring all possible participants in a swap. In the case of contrib.auth: Auth requires the concept of a model that represents a User. At the end of the day, it shouldn't matter which model that user actually is -- it just needs to be a model, and we can determine at runtime what that model should be. You don't have to predeclare that you're going to be used as a User - you just point to the model using AUTH_USER_MODEL. For ease of use (and backwards compatibility) we need to ship a concrete User model. But then we need to be able to: * Not install the model if it's been replaced, and * Easily answer the runtime question "what have you been swapped out for?" This is done by checking Meta.swappable, reading the value of the setting, and checking if Meta.model_label matches the value in the setting. This is cached as Meta.swapped. The internals of Django (like syncdb) then check Meta.swapped as part of the checks performed to see if a model should be included in any given activity. I've had a bit of a closer look at the implementation with your use case in mind, and I'm less concerned about land mines caused your proposed usage of swappable. It works by happy accident :-) However, I'd still make the argument that while it's *possible* to use swappable in this way, it's still not desirable. Firstly, it leads to inconsistent usage of API. Under the current scheme, the default User model defines swappable. No other User model needs to. In your proposed usage, you've just made a special case of "Swappable models in the same app". Now, granted - this discrepancy isn't something that needs to be documented (because swappable isn't documented), but if someone goes looking, or if we ever want to formalise swappable (which is certainly my long term hope), we need to explain why some models need swappable defined and others don't. Alternatively, if we change the game to say that swappable *is* required on all models, then we have a backwards compatibility problem (because no existing custom user model needs swappable to be defined). *Any* model can be swapped in as a substitute user. If we were to go down this path, the logical extension (to my mind) would be to validate that you're not swapping in a model that hasn't declared itself as swappable (otherwise, why would you go to the trouble of predeclaring?). This *isn't* currently being done, and I'm not convinced it has value. This is an instance where duck typing works really well. There's also the example I raised about comments on User - you aren't necessarily in control of the model that you want to use as a swappable alternative. Secondly, marking a model as swappable doesn't make it zero cost. It's still parsed, loaded and added to the app cache, even if it isn't used. Django's internals then ignore any model that is marked as swapped. If we put EmailUser in contrib.auth, it's *always* going to be part of every project that uses auth, regardless of whether you're using the default User, EmailUser, or a third party User app. That's overhead that isn't present if EmailUser is in a separate opt-in app. I'm willing to accept the overhead for the single endpoint User model, because it's the origin - the default thing that needs to be swapped out. I'm less inclined to say that this is acceptable for all possible alternative User models that might be included in auth. Lastly, we have consistency with external usage, and the benefits of documentation by example. Current usage says that use a custom User model, you add the app the INSTALLED_APPS and set AUTH_USER_MODEL. By shipping email_auth as a separate app, we'd be continuing the same advice, and the app would serve as a helpful example for how to do it. We'd be eating our own dog food/leading by example. So far, the only convincing argument I've heard for putting the EmailUser in contrib.auth is that it avoids having to add a value to INSTALLED_APPS. And I'm afraid that, by itself, just doesn't convince me. Everything I see suggests that a standalone app is cleaner and more explicit, and serves as a better example to the wider community. I just don't consider an extra entry in INSTALLED_APPS -- explicitly saying "I want to have access to this extra model", the same way you do for *every* other Django app -- is onerous. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
