On Sep 19, 2013, at 11:15 PM, Russell Keith-Magee <[email protected]> wrote:
> 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. If we went with this method, the explanation would be that a model must define itself as swappable if the intent is for it to be able to be overridden. So, the idea would be that any user model that we chose to ship in Django should be marked swappable, but the User model that someone wrote for their own app would not be. Even if we decide to go with an auth_email app, we should almost certainly still mark that User model as swappable. (What if someone wanted the forms and not the model?) > 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. I completely agree. Any setup that requires swappable on all models does not make sense. But simply marking swappable on models that *we* ship that are intended to be able to be subbed out does make sense, and doesn't cause back-compat issues. Right now Django ships one model: User, marked swappable. By declaring your own model and setting it as AUTH_USER_MODEL, the swappable setting makes it so that ours is not used. I am proposing that Django would ship two models: User and EmailUser, both marked swappable. If you declare your own, neither of ours are used. If you use one of ours, the other is not used. If you go the declare your own route, there is no need to mark your model as swappable. It is, as you state, a regular model. > 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. I agree here too. I don't think this idea has value. However, I don't think it's a necessary condition, either. > 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. I have two reasons for wanting this method: one is the public API, but the other (maybe bigger) is code duplication. Django is about DRY, after all. If we do an auth_email model, we have to largely duplicate the model, and the form, and any relevant views. If we ship an email user model, what seems the path of least resistance to me would be to have the existing User subclass EmailUser, add the username field and redefine USERNAME_FIELD and REQUIRED_FIELDS. Things like (for example) UserCreationForm would be defined to have the two password fields and then have the username field derived from the field that is declared as USERNAME_FIELD. This has the expedient side effect of making it useful for more substitution models in the wild, although obviously not all of them. You've been referring to a reference implementation as a valuable aspect of the separate-app route, but one of the virtues of my proposal is that it increases the number of cases where much less has to be implemented at all. tanderegg's implementation really hits this home for me: he has models, forms, etc. that all look almost-but-not-quite-exactly like the ones already being shipped. That also means double maintenance -- if we want to maintain some aspect that's duplicated between them, we have to maintain it in both spots, to keep them in sync. I've gone a little ways down toward implementing both paths, and one thing that is clear to me is that the code required to ship an alternate model within the same app is substantially less, and has much less (if any) duplication involved. L P. S. I'm not the sharpest knife in the drawer when it comes to social things, so I want to state explicitly: I am continuing to debate the question because I perceive the debate to be moving in a useful way. If I'm, in fact, simply being confrontational, then tell me to shut up and I'll just work on doing it your way / commenting on tanderegg's start. At the end of the day, I recognize that it is your decision to make, and at the end of the day, if I can't convince you, I'll accept it with a smile and get the work done! I definitely want to be offering a positive contribution. -- 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 [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. For more options, visit https://groups.google.com/groups/opt_out.
