On 19 janv. 2014, at 08:56, Russell Keith-Magee <russ...@keith-magee.com> wrote:
> This is fine, but it limits the logic that we (as a project) can put into > AdminConfig.ready(), because existing projects won't reference the new > AdminConfig. So, cleanups like putting admin.autoregister and check framework > registrations into the ready function can't be done, because existing > projects won't automatically gain that functionality. > > So, we end up with a situation where we have a nifty new on-startup method, > but we can't use it for any of our own on-startup app requirements, because > we have no guarantees that it's going to be invoked. I would take a more moderate position. We can certainly use AppConfig.ready() for new features such as the checks framework. It’s reasonable to ask our users to edit their configuration to take advantage of a new feature. If they don’t, they don’t lose anything compared to the previous version of Django. Backwards-compatibility is preserved. We can also use it for optional features such as admin.autodiscover(). Putting it in urls.py still works, but it’s simply better to use AdminConfig. The release notes explain the upgrade path. It users don’t follow them, they don’t lose anything. If we wanted to move a major existing feature to AppConfig.ready(), and stop supporting the previous implementation, then we’d have to implement a deprecation path or, depending on the context, document it as a backwards-incompatible change. But this situation still has to happen. As far as I can tell, we’re talking about an hypothetical problem. > Assuming that this wasn't done for import-related reasons, I can see a couple > of possible solutions: > > 1) Require updating INSTALLED_APPS as a migration step. This is something we > can detect with a check migration command - we inspect all the strings in > INSTALLED_APPS; if it isn't an AppConfig subclass, look to see if there is an > apps module, and if so, suggest that an update may be required. > > Benefits - explicit upgrade path. > Downside - if you ignore the warning, you potentially lose functionality. And > nobody ever reads the documentation :-) It really depends how much we want to push the use of AppConfig. At the moment the consensus among the core devs I’ve spoken with is “wait and see”. That means not making the feature too prominent in 1.7 and adding suitable documentation in 1.8, depending on how well it works. Also there might be good reasons for not using an AppConfig. > 2) Use a naming convention - require that the default app configuration must > be called AppConfig, so we can always try to load myapp.apps.AppConfig. If > this object doesn't exist, install the system default AppConfig (as it does > currently). > > Benefits - Easy to implement and document the requirement. > Downside - It's coding by convention; also, every app config is called > AppConfig, which is potentially painful for debugging. Jannis vetoes that idea and I believe he’s right. Auto-importing modules for discovery is a bad idea. “One does not simply import modules.” ;-) > 3) Do 2, but with a level of indirection. Let the user call the AppConfig > module whatever they want, but allow the apps module to define a > 'default_app_config' attribute that provides the string to use as a default > AppConfig. > > This is essentially halfway between 1 and 2 - it's implicitly doing the > INSTALLED_APPS update, using information provided by the app developer. > > 3a) As for 3, but use a flag on the AppConfig subclass that marks it as "use > me as the default". If there are more than one AppConfig objects in an apps > module that has the 'use as default" flag, raise an error. > > 3b) As for 3, but use the first class discovered that is a subclass of > AppConfig. Actually identifying the "first" is a bit of an implementation > issue - I suppose we'd need to have a MetaClass that registered AppConfig > objects as they are imported. All these still suffer from the implicit import problem. I chose not to perform any kind of discovery because explicit > implicit To avoid that, we could put default_app_config in the application’s root package but that’s ugly. -- Aymeric. -- 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. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CA318055-63E5-43EC-9F5F-A9A6E326C381%40polytechnique.org. For more options, visit https://groups.google.com/groups/opt_out.
