App-loading: Pragmatic concerns about default AppConfig objects and ready() implementations

[Russell Keith-Magee]

Hi all,

First off - this isn't critical for 1.7 alpha - I'm raising it now because I've been in this space for the last couple of days as a result of the working on the check framework. I suspect any changes stemming from this discussion can be landed in the beta period without major problems.

Also - Aymeric - you've done a great job with this app loading work. I didn't get much of a chance to follow along as it was unfolding, but the resultant work has been a pleasure to work with.

With one exception :-)

We've now got AppConfig objects for each app (e.g., contrib.admin). Those AppConfig objects can have ready() methods that invoke startup logic. This is all great stuff that has been a long time coming.

*But*

From a backwards compatibility perspective, there seems to be a gap in the way AppConfig objects are instantiated.

The gap is this - historical apps (and historical documentation) calls for admin to be put into INSTALLED_APPS using 'django.contrib.admin'. However, when you do this, you get the system default AppConfig, not something that is admin specific. In order to get the new AdminConfig, you need to update your INSTALLED_APPS to point at "django.contrib.admin.apps.AdminConfig". 

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've dug through the mailing lists discussions about app-loading, and I can't see anywhere that this was explicitly discussed, so I can't tell if this was done deliberately, or as a side effect of the circular import problems that Aymeric described.

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 :-)

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.

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. 

There's probably a bunch of other options I haven't thought of.

Any thoughts from anyone about this (in particular, Aymeric, since you're most familiar with the internals)?

Yours,

Russ Magee %-)

[Aymeric Augustin]

On 19 janv. 2014, at 08:56, Russell Keith-Magee <rus...@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.

[Marc Tamlyn]

All of your proposed solutions require trying to import an apps.py module which may not exist. I know Aymeric was very much against this, especially as python imports have potential side effects, and we don't know what people might have in a apps module already.

Anyways, configuration over convention and explicit better than implicit etc.

It does mean that we can't put any required logic into AppConfig, but for example admin.autodiscover is fine as existing projects already call it in their urls.py (or wherever they moved it to), so there is no concern here, especially if we update the project template.

The only other obvious contrib thing which would be nice to move is signal registration, but this isn't that necessary.

The hope is that people will migrate towards using AppConfig in their settings, and then we can reconsider. Third party apps who wish to do a lot of logic there may wish to require it.

Personally I'm torn, as I can see the benefits of a convention. But equally I don't think there's a huge amount of point making a big effort to rewrite Django's import handling in order to add more magic back in.

Marc

--
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-develop...@googlegroups.com.
To post to this group, send email to django-d...@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/CAJxq849Hx24ETnugK54xGGRpQNUFON18YSwPdXKv0fYg%2B7tnxQ%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Russell Keith-Magee]

On Sun, Jan 19, 2014 at 5:48 PM, Aymeric Augustin <aymeric....@polytechnique.org> wrote:

On 19 janv. 2014, at 08:56, Russell Keith-Magee <rus...@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.

I'm afraid I don't see *how* we can use ready() for new features. Checks are *definitely* off the table for using the feature. Take Auth, for example. We need to register the various checks related to AUTH_USER_MODEL. We need to register these checks regardless of whether you're using django.contrib.auth or django.contrib.auth.apps.AuthApp. So we have to put the registration somewhere it will be picked up by the 'django.contrib.auth' usage; at which point, any usage in ready() is either redundant, or requires a logic check along the lines of "if you were added to INSTALLED APPS using an AppConfig, don't register", 

The same goes for admin and contenttypes (the two other contrib apps that gain checks modules). 

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.

I'll certainly grant this one - auto discover is something that's been "opt-in" (of a fashion), so we can tell people to just stop using it (and/or change the project template). 

 

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.

Right - which is effectively a variant on Option 1 - treat this as a "you need to update" problem, and provide a deprecation path and upgrade compatibility check as assisting tools.

 

As far as I can tell, we’re talking about an hypothetical problem.

 

It's not hypothetical at all - I actually can't implement the checks framework using the ready() method, because that logic path won't be executed. I have to use an import side effect in the __init__ module of contrib.auth, contrib.contenttypes and contrib.admin. This is less than ideal, but it's not something were we have any other option, AFAICT -- hence my enthusiasm about using ready().

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.

I suppose I've been around long enough wishing we had App objects that I can't wait to see them in active use :-)

Pragmatically, I can appreciate the take-it-slow approach. 

 

Also there might be good reasons for not using an AppConfig.

 

I'd be interested in hearing what these are. Having a stable and predictable place for signal registration is worth the price of admission alone, IMHO. I've got plenty of things that I'd like to start experimenting with around AppConfig objects.

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.” ;-)

I'll certainly appreciate (and largely agree with) that logic. My concern is the pragmatic one of not being able to use ready() as a clean approach to a problem I'm currently solving in a less than ideal way.

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

 

I can certainly accept that as a motivation.

To avoid that, we could put default_app_config in the application’s root package but that’s ugly.

I agree that it's less than ideal, but I don't think a string definition in __init__is *more* ugly than implicit imports and registrations happening in the __init__  - in fact, I'd say it's a whole lot *less* ugly, because it's just identifying a string in a module that was already being imported, and using that as a trigger to import another specific module (one that the end user could reference explicitly if they were so inclined).

Yours,

Russ Magee %-)

[Russell Keith-Magee]

HI Marc,

On Sun, Jan 19, 2014 at 5:52 PM, Marc Tamlyn <marc....@gmail.com> wrote:

All of your proposed solutions require trying to import an apps.py module which may not exist. I know Aymeric was very much against this, especially as python imports have potential side effects, and we don't know what people might have in a apps module already.

Anyways, configuration over convention and explicit better than implicit etc.

It does mean that we can't put any required logic into AppConfig, but for example admin.autodiscover is fine as existing projects already call it in their urls.py (or wherever they moved it to), so there is no concern here, especially if we update the project template.

The only other obvious contrib thing which would be nice to move is signal registration, but this isn't that necessary.

The hope is that people will migrate towards using AppConfig in their settings, and then we can reconsider. Third party apps who wish to do a lot of logic there may wish to require it.

Personally I'm torn, as I can see the benefits of a convention. But equally I don't think there's a huge amount of point making a big effort to rewrite Django's import handling in order to add more magic back in.

See my comments to Aymeric. I'm not blind to the concerns you've raised here - there certainly valid ones, and I appreciate (and sympathise with) the intent. I'm just stuck in a bind where there's a new feature that will fix my problem much better than the existing alternatives -- I just can't use it.

I'm completely open to other approaches that don't require speculative imports. Aymeric's suggestion of an attribute in the root module sounds like a good option to me -- it only requires an attribute access in a module that you're already importing, and you use that string to provide the value that we'd like to tell the end user to put in their INSTALLED_APPS. However, it sounds like Aymeric wasn't as keen on this approach.

Yours,

Russ Magee %-)

[Marc Tamlyn]

For what it's worth, if we have some code that:

a) is a new feature we want to ensure runs
b) has no obvious, explicit place to place it
and
c) is trivially solved by .ready()

Then I start to think that practicality beats purity. Personally, I think we should still encourage the "explicit" approach where we recommend to users that they use 'path.to.AppConfig' and maybe one day deprecate not having an AppConfig - for external apps at least this ensures translatable admin, but in the mean time add an impure convention (which if we are going to do, I don't care which one we do).

Explicit > Implicit yes, but if our choice is 'introduce an implicit convention, which we could one day deprecate' vs 'add some more import time side effect code' then I personally think we should go for the former.

In Django 2.0 we do away with all the implicit importing and searching and you have to configure your AppConfig instances for management commands, template directories, static dirs, models, admin config... ;)

Marc

--
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-develop...@googlegroups.com.
To post to this group, send email to django-d...@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/CAJxq84_MPKw_zywaeaQ-4aXDYaTVJxfRtm%2B-2GEXrrn7D7VwJQ%40mail.gmail.com.

[Russell Keith-Magee]

On Sun, Jan 19, 2014 at 9:52 PM, Marc Tamlyn <marc....@gmail.com> wrote:

For what it's worth, if we have some code that:

a) is a new feature we want to ensure runs
b) has no obvious, explicit place to place it
and
c) is trivially solved by .ready()

Then I start to think that practicality beats purity. Personally, I think we should still encourage the "explicit" approach where we recommend to users that they use 'path.to.AppConfig' and maybe one day deprecate not having an AppConfig - for external apps at least this ensures translatable admin, but in the mean time add an impure convention (which if we are going to do, I don't care which one we do).

Explicit > Implicit yes, but if our choice is 'introduce an implicit convention, which we could one day deprecate' vs 'add some more import time side effect code' then I personally think we should go for the former.

The other option (which was option 1, and seems to have been overlooked) was to not do anything implicit, and make it a hard upgrade path using the checks framework itself. We can have compatibility checks that explicitly look for the contrib app strings in INSTALLED_APPS, and raise an error to the effect that "your app isn't being fully validated". Core compatibility checks aren't tied to apps embedded in the system, so they always run, so you wouldn't be able to do a syncdb/migrate or runserver without hitting an error that has hint telling you want you need to do. The migration itself is simple -- modify three lines in your INSTALLED_APPS, and you never see it again. It's just a question of whether we'd prefer a hands-off migration using an implicit technique, or a hands-on migration pointed to by the checks framework.

In Django 2.0 we do away with all the implicit importing and searching and you have to configure your AppConfig instances for management commands, template directories, static dirs, models, admin config... ;)

Oh, that mystical land of Django 2.0, where the sun shines shiner … :-)

Russ %-)

[Aymeric Augustin]

On 19 janv. 2014, at 13:46, Russell Keith-Magee <rus...@keith-magee.com> wrote:

On Sun, Jan 19, 2014 at 5:48 PM, Aymeric Augustin <aymeric....@polytechnique.org> wrote:

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.

I'm afraid I don't see *how* we can use ready() for new features. Checks are *definitely* off the table for using the feature.

Oops, I missed that the checks framework replaces *existing* validation features. Sorry.

Also there might be good reasons for not using an AppConfig.

 

I'd be interested in hearing what these are.

For instance, if you’re using a custom AdminSite, you may want the admin’s auto-discovery not to kick in. Honestly, it’s a theoretical argument, I’m not sure it really matters in practice.

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). 

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.” ;-)

I'll certainly appreciate (and largely agree with) that logic. My concern is the pragmatic one of not being able to use ready() as a clean approach to a problem I'm currently solving in a less than ideal way.

In fact, that was part of the original implementation. My main motivation was to make the transition easier for pluggable apps, and especially contrib apps. That’s also what you’re looking for right now.

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. 

we could put default_app_config in the application’s root package but that’s ugly.

I agree that it's less than ideal, but I don't think a string definition in __init__is *more* ugly than implicit imports and registrations happening in the __init__  - in fact, I'd say it's a whole lot *less* ugly, because it's just identifying a string in a module that was already being imported, and using that as a trigger to import another specific module (one that the end user could reference explicitly if they were so inclined).

Indeed, this convention provides a way for pluggable apps to select a default AppConfig class without requiring end-users to change INSTALLED_APPS.

It would make the transition much easier. For instance, at the moment, there’s no way to configure the debug toolbar in a way that works both on 1.6 and 1.7.

I’m at least +0, and probably +1 on the idea, but I’d like to hear the opinion of other people who contributed to the current design, especially Jannis.

-- 

Aymeric.

[Russell Keith-Magee]

For tracking purposes, I've opened a ticket:

https://code.djangoproject.com/ticket/21829

Russ %-)

--
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-develop...@googlegroups.com.
To post to this group, send email to django-d...@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/45F925BB-699F-4D7D-9871-ACD766E85926%40polytechnique.org.

[Jannis Leidel]

On 19.01.2014, at 08:56, Russell Keith-Magee <rus...@keith-magee.com> wrote:

> Hi all,
>
> First off - this isn't critical for 1.7 alpha - I'm raising it now because I've been in this space for the last couple of days as a result of the working on the check framework. I suspect any changes stemming from this discussion can be landed in the beta period without major problems.
>
> Also - Aymeric - you've done a great job with this app loading work. I didn't get much of a chance to follow along as it was unfolding, but the resultant work has been a pleasure to work with.
>
> With one exception :-)
>
> We've now got AppConfig objects for each app (e.g., contrib.admin). Those AppConfig objects can have ready() methods that invoke startup logic. This is all great stuff that has been a long time coming.
>
> *But*
>
> From a backwards compatibility perspective, there seems to be a gap in the way AppConfig objects are instantiated.
>
> The gap is this - historical apps (and historical documentation) calls for admin to be put into INSTALLED_APPS using 'django.contrib.admin'. However, when you do this, you get the system default AppConfig, not something that is admin specific. In order to get the new AdminConfig, you need to update your INSTALLED_APPS to point at "django.contrib.admin.apps.AdminConfig".
>
> 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.
>

Hi Russ, all,

Thanks for raising this issue as it’s indeed an important one to discuss. I hoped we’d have time to let the community figure out some of the techniques to handle version compatibility to Django but alas I didn’t expect the checks framework to so thoroughly crash the party. Well done! :) I wish we would have had more time to take these points into account when we were discussing the various details back in December, but I guess as long as 1.7 isn’t out it’s not too late. I probably would have made the argument that the check system may be best placed in AppConfig classes (e.g. in AppConfig.check_* methods). But on the other hand that may have made the work on the app config classes even harder.

For the record, the app config concept has a long history with different implementations and goals but always (AFAIR) tried to fix one thing: the side effects of the implicit Django app concept. This issue is just another example of that problem. It bubbles up in our compatibility strategy which is to still allow dotted app module paths in INSTALLED_APPS. We assumed users to upgrade to the dotted app config paths in INSTALLED_APPS on their own eventually if the app in question required it. In other words, we really left it to the app authors to decide when to ask their users to make the jump. Instead of a hard coded check we wanted to gently push the new APIs into the community — also to find concept holes.

I now believe that we may have been too conservative since that strategy introduces a new level of uncertainty in the app API. I can see the user questions already: “Does the 3rd party app XYZ work on 1.7?” “Can I also install it on my legacy 1.6 project?” “How am I going to override some options without a AppConfig?” “What’s the best way to create cross version compatible apps?”

> I've dug through the mailing lists discussions about app-loading, and I can't see anywhere that this was explicitly discussed, so I can't tell if this was done deliberately, or as a side effect of the circular import problems that Aymeric described.
>
> 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 :-)

+1 It’s the strictest and but also clearest migration path and I believe we can’t afford to fix this over time and leave our users in limbo about how Django apps work or what to expect from the app API. It means it’ll be harder for reusable app developers to explain backward compatibility to their users but we could offer some pointers, e.g. in blog/models.py::

try:
from django import apps
except ImportError:
from blog.utils import register_something
register_something()

That would work for Django versions < 1.7 and implies blog.utils.register_something is called in the ready() method of blog.apps.BlogConfig, too. It’s not an exciting pattern but at least common enough for Python developers to require little explanation.

> 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.

Yeah, this is a bad idea. I love conventions, but not if they prevent me to paint my shed blue. -1

> 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.

While I understand that it would allow us to keep using dotted module paths in INSTALLED_APPS, it’s also a great way to stall the adoption of app config classes. To be clear: it *is* the goal to eventually have all apps (contrib and 3rd party) use app configs as the only location for startup code and app configuration. App config classes are intended to be user and not only app developer facing. We strive to encourage users to be app developers as soon as possible, e.g. we tell them how to write an app in the tutorial. So introducing a hidden configuration variable like ‘default_app_config’ makes me really uncomfortable with regard to the mixed message we’d be sending. We should be clear about what app is used. The “default app config” is the one that is configured in the settings.py, just like the “default database backend” is the one defined there. In other words it’s the users choice what is considered default, not ours or the app developer’s.

> 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.

This smells like dependency tracking and hard to explain errors. I also don’t see how a user could prevent 3rd party apps to not mark itself as default. -1

> 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.

More implicit behavior that reminds me of how the model implementation made it so hard to have pluggable user models. -1

Jannis

[Carl Meyer]

On 01/19/2014 07:08 AM, Aymeric Augustin wrote:
> On 19 janv. 2014, at 13:46, Russell Keith-Magee <rus...@keith-magee.com

> <mailto:rus...@keith-magee.com>> wrote:
>
>> On Sun, Jan 19, 2014 at 5:48 PM, Aymeric Augustin
>> <aymeric....@polytechnique.org

>> <mailto:aymeric....@polytechnique.org>> wrote:
>>
>> Also there might be good reasons for not using an AppConfig.
>>
>> I'd be interested in hearing what these are.
>

> For instance, if you�re using a custom AdminSite, you may want the
> admin�s auto-discovery not to kick in. Honestly, it�s a theoretical
> argument, I�m not sure it really matters in practice.

I have several times opted out of admin.autodiscover() in favor of
manual registration of models to multiple different custom admin sites.
Honestly admin.autodiscover() and the usual way of doing admin
registration as an import-time side effect of app/admin.py is ugly,
although admittedly convenient for the common case. I very much hope
that moving the admin autodiscovery to the admin AppConfig still allows
some reasonable path to opt-out of autodiscover - perhaps by subclassing
the admin AppConfig?

This is probably all a tangent to this discussion, though - if "subclass
the AppConfig and override something" works to opt out of autodiscover,
then I still wouldn't see this as a "good reason for not using an
AppConfig."

Carl

[Aymeric Augustin]

On 20 janv. 2014, at 18:36, Carl Meyer <ca...@oddbird.net> wrote:

> I very much hope
> that moving the admin autodiscovery to the admin AppConfig still allows
> some reasonable path to opt-out of autodiscover - perhaps by subclassing
> the admin AppConfig?

We could provide AdminAutoDiscoveryConfig and AdminConfig.

The alternative is to modify INSTALLED_APPS to support passing
arguments to the AppConfig class. But I find it rather ugly.

> This is probably all a tangent to this discussion, though - if "subclass
> the AppConfig and override something" works to opt out of autodiscover,
> then I still wouldn't see this as a "good reason for not using an

> AppConfig.”

It works but I wouldn’t encourage such use. What if AdminConfig.ready()
does three things and you want only two? What if the next release adds a
fourth thing?

--
Aymeric.

[Carl Meyer]

On 01/20/2014 12:22 PM, Aymeric Augustin wrote:
> On 20 janv. 2014, at 18:36, Carl Meyer <ca...@oddbird.net> wrote:
>
>> I very much hope
>> that moving the admin autodiscovery to the admin AppConfig still allows
>> some reasonable path to opt-out of autodiscover - perhaps by subclassing
>> the admin AppConfig?
>
> We could provide AdminAutoDiscoveryConfig and AdminConfig.
>
> The alternative is to modify INSTALLED_APPS to support passing
> arguments to the AppConfig class. But I find it rather ugly.
>
>> This is probably all a tangent to this discussion, though - if "subclass
>> the AppConfig and override something" works to opt out of autodiscover,
>> then I still wouldn't see this as a "good reason for not using an

>> AppConfig.�
>
> It works but I wouldn�t encourage such use. What if AdminConfig.ready()

> does three things and you want only two? What if the next release adds a
> fourth thing?

Sure - for this to be a "reasonable" approach the autodiscovery would
need to be done in a purpose-specific method called by ready(), not
directly in ready(). Then one could just override that method to be a no-op.

But providing both app configs is even better!

Carl

[Carl Meyer]

Hi all,

I just filed #21831, which is a release-blocking regression directly
related to this. My opinion is that doing check-registration as an
import side-effect in `__init__.py` is not viable, as it means that
every single check needs to be coded defensively against the possibility
that the app isn't actually installed (which, evidence suggests, doesn't
come to mind naturally.)

So that means we need to pick some solution that allows us to rely on
AppConfig.ready() always being called, at least for contrib apps. My
thoughts on the specific options below:

On 01/20/2014 08:09 AM, Jannis Leidel wrote:
> On 19.01.2014, at 08:56, Russell Keith-Magee

>> 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 :-)
>

> +1 It�s the strictest and but also clearest migration path and I
> believe we can�t afford to fix this over time and leave our users in

> limbo about how Django apps work or what to expect from the app API.

> It means it�ll be harder for reusable app developers to explain

> backward compatibility to their users but we could offer some
> pointers, e.g. in blog/models.py::
>
> try: from django import apps except ImportError: from blog.utils
> import register_something register_something()
>
> That would work for Django versions < 1.7 and implies
> blog.utils.register_something is called in the ready() method of

> blog.apps.BlogConfig, too. It�s not an exciting pattern but at least

> common enough for Python developers to require little explanation.

I'm -0 for this option. If we were really _requiring_ users to update to
AppConfigs in INSTALLED_APPS immediately, that would be a violation of
our backwards-compat policy, obviously. But since at this point we're
just talking about users losing the validation features, which aren't
production-critical, and we're providing a warning - I'd be ok with
this. On the other hand, as you point out, it makes it much more
difficult for reusable app authors to begin using AppConfig.ready() for
required setup, which I think will be a major roadblock to adoption of
AppConfig; I'm not sure why we'd want that. Basically for any reusable
app author who has a similar back-compat policy to Django's, they would
have to wait several releases before they could rely on AppConfig.ready().

>> 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.
>
> Yeah, this is a bad idea. I love conventions, but not if they prevent
> me to paint my shed blue. -1

I agree; -1 to naming-based conventions.

>> 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.
>
> While I understand that it would allow us to keep using dotted module

> paths in INSTALLED_APPS, it�s also a great way to stall the adoption

> of app config classes. To be clear: it *is* the goal to eventually
> have all apps (contrib and 3rd party) use app configs as the only
> location for startup code and app configuration. App config classes
> are intended to be user and not only app developer facing. We strive
> to encourage users to be app developers as soon as possible, e.g. we
> tell them how to write an app in the tutorial. So introducing a

> hidden configuration variable like �default_app_config� makes me
> really uncomfortable with regard to the mixed message we�d be
> sending. We should be clear about what app is used. The �default app
> config� is the one that is configured in the settings.py, just like
> the �default database backend� is the one defined there. In other
> words it�s the users choice what is considered default, not ours or
> the app developer�s.

I agree that explicit configuration via INSTALLED_APPS is a worthy goal,
but I think this option provides by far the best migration path
available. I think "default_app_config" will *accelerate* the adoption
of AppConfig classes, because it allows app developers (like us, with
django.contrib!) to begin immediately and fully making use of AppConfig
features (which clearly we need) without requiring immediate updates
from users.

If we want to, we could later deprecate default_app_config in favor of
requiring explicit configuration in INSTALLED_APPS. This allows us to
eventually reach the same goal as option 1, but with a much less
disruptive migration.

I don't believe that it sends a "mixed message" to introduce a new
feature, but provide a migration path towards using that feature which
is as smooth as possible for users. On the contrary, I think it's the
right thing to do for our users.

+1 to this option. (Though I think default_app_config should be
specified in the app module's "__init__.py", not in "apps.py", to avoid
introducing any new auto-import of a module not explicitly referenced in
settings.py.)

>> 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.
>
> This smells like dependency tracking and hard to explain errors. I

> also don�t see how a user could prevent 3rd party apps to not mark
> itself as default. -1

I don't see what advantage this has over (3) - it just allows another
failure mode. -0.

>> 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.
>
> More implicit behavior that reminds me of how the model
> implementation made it so hard to have pluggable user models. -1

Agreed; absolutely no way we make this implicitly dependent on order of
imports. -!.

Carl

[Carl Meyer]

On 01/20/2014 01:01 PM, Carl Meyer wrote:
>>> 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.
>>
>> This smells like dependency tracking and hard to explain errors. I
>> also don�t see how a user could prevent 3rd party apps to not mark
>> itself as default. -1
>
> I don't see what advantage this has over (3) - it just allows another
> failure mode. -0.

I just realized that this option also requires adding an automatic
attempt to import the "apps" submodule within an app, when the "apps"
submodule was not explicitly referenced in settings (effectively making
the "apps" submodule a magic name, and potentially causing immediate
issues for apps that already have an apps.py for whatever reason.)

So count me as -1 on this option, not -0. (And I'd be similarly -1 on
the "default_app_config", if it were required to be defined in apps.py
rather than __init__.py, for the same reason. But I'm still +1 on it if
it's defined in __init__.py.)

Carl

[Marc Tamlyn]

Now I have a moment to mention this, there is a very important issue I currently don't see a way around, contrib highlights it well but in reality it's more a third party app issue.

Suppose my third part app has a (normal) Django two-release support principle. Django 1.8 is coming out and although I backed out of using AppConfigs with 1.7 because things were awkward and I preferred to leave the hacks, I'm not going to "do it right" and use AppConfigs. My app has required things which must run in ready() or there will be strange and cryptic errors. There's no obvious way to detect that they haven't been run at the point of use, or this may be too late (for example modifying a model). I update all my documentation to say "you must use the AppConfig now", but I still get 50 users email me complaining that they have cryptic error messages and the whole thing is broken.

I personally think that we need some way of telling Django that it *must* use a given AppConfig (unless told otherwise) for an application. I'm quite happy for that method to always throw warnings, perhaps after deprecation throw errors, but I must be able to specify that it needs an AppConfig to work correctly, ideally in such a way that it will tell you what you need to do.

Now, personally I don't *like* default_app_config. It smells ugly, and I don't want people to view it as a long term solution. So how about this twist on the suggestion:

- Some applications do not require a customised AppConfig, they do not need to do anything. They may have one (e.g. contrib.sites?) but it doesn't provide anything other than customisation hooks.

- Some applications require an AppConfig. They can set required_app_config = 'myapp.apps.MyAppConfig' in myapp.__init__. This will be picked up if the user has put 'myapp' in INSTALLED_APPS, and will be used. In Django 1.7, this will throw a DeprecationWarning, and say which app is affected and what the default path should be. In Django 1.8 it will error during django.startup(). The ability to specify required_app_config in this way stays in Django until at least 1.9 if not forever, but which point hopefully we will be near to 100% adoption of AppConfigs in the wild.

- We update the docs, tutorial and project template to use AppConfigs all the time, and we advertise them as much as possible.

We can now move all the new check code into AppConfig.ready(), and anything else we want. We now have a way of ensuring that they *will* get run without users updating their code, and we're shouting as loudly as we dare to the users to tell them what's going on.

Thoughts?

Marc

[PS, I've opted for accelerated timings on this update path, which I justify on the basis that by using required_app_config we are in a sense changing the configuration of an app that the user has in their settings, therefore a visible warning is justified. However, I'm +0 on doing this over an extra release cycle]

On 20 January 2014 20:09, Carl Meyer <ca...@oddbird.net> wrote:

On 01/20/2014 01:01 PM, Carl Meyer wrote:
>>> 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.
>>
>> This smells like dependency tracking and hard to explain errors. I

>> also don’t see how a user could prevent 3rd party apps to not mark

>> itself as default. -1
>
> I don't see what advantage this has over (3) - it just allows another
> failure mode. -0.

I just realized that this option also requires adding an automatic
attempt to import the "apps" submodule within an app, when the "apps"
submodule was not explicitly referenced in settings (effectively making
the "apps" submodule a magic name, and potentially causing immediate
issues for apps that already have an apps.py for whatever reason.)

So count me as -1 on this option, not -0. (And I'd be similarly -1 on
the "default_app_config", if it were required to be defined in apps.py
rather than __init__.py, for the same reason. But I'm still +1 on it if
it's defined in __init__.py.)

Carl

--
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-develop...@googlegroups.com.
To post to this group, send email to django-d...@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/52DD8289.9010808%40oddbird.net.

[Tim Chase]

n 2014-01-20 20:22, Aymeric Augustin wrote:
> The alternative is to modify INSTALLED_APPS to support passing
> arguments to the AppConfig class. But I find it rather ugly.

[I've only been lurking in this thread, so take with a grain of salt;
just throwing it out there to see what sticks]

Would it be possible to do something like

VALIDATION_APPS = SomeComplexValidatorWithMetadata(
app1=...,
app2=...,
)

INSTALLED_APPS = VALIDATION_APPS.get_installed_apps()

which would allow a more complex descriptor for validation purposes,
but then also keep things fairly DRY so you don't have apps
duplicated between the two settings and the INSTALLED_APPS remains
backwards compat.

-tkc

[Carl Meyer]

Hi Marc,

On 01/20/2014 01:23 PM, Marc Tamlyn wrote:
> Now I have a moment to mention this, there is a very important issue I
> currently don't see a way around, contrib highlights it well but in
> reality it's more a third party app issue.
>
> Suppose my third part app has a (normal) Django two-release support
> principle. Django 1.8 is coming out and although I backed out of using
> AppConfigs with 1.7 because things were awkward and I preferred to leave
> the hacks, I'm not going to "do it right" and use AppConfigs. My app has
> required things which must run in ready() or there will be strange and
> cryptic errors. There's no obvious way to detect that they haven't been
> run at the point of use, or this may be too late (for example modifying
> a model). I update all my documentation to say "you must use the
> AppConfig now", but I still get 50 users email me complaining that they
> have cryptic error messages and the whole thing is broken.
>
> I personally think that we need some way of telling Django that it
> *must* use a given AppConfig (unless told otherwise) for an application.
> I'm quite happy for that method to always throw warnings, perhaps after
> deprecation throw errors, but I must be able to specify that it needs an
> AppConfig to work correctly, ideally in such a way that it will tell you
> what you need to do.

I agree.

> Now, personally I don't *like* default_app_config. It smells ugly, and I
> don't want people to view it as a long term solution. So how about this
> twist on the suggestion:
>
> - Some applications do not require a customised AppConfig, they do not
> need to do anything. They may have one (e.g. contrib.sites?) but it
> doesn't provide anything other than customisation hooks.
> - Some applications require an AppConfig. They can set
> required_app_config = 'myapp.apps.MyAppConfig' in myapp.__init__. This
> will be picked up if the user has put 'myapp' in INSTALLED_APPS, and
> will be used. In Django 1.7, this will throw a DeprecationWarning, and
> say which app is affected and what the default path should be. In Django
> 1.8 it will error during django.startup(). The ability to specify
> required_app_config in this way stays in Django until at least 1.9 if
> not forever, but which point hopefully we will be near to 100% adoption
> of AppConfigs in the wild.
> - We update the docs, tutorial and project template to use AppConfigs
> all the time, and we advertise them as much as possible.
>
> We can now move all the new check code into AppConfig.ready(), and
> anything else we want. We now have a way of ensuring that they *will*
> get run without users updating their code, and we're shouting as loudly
> as we dare to the users to tell them what's going on.

This seems the same as the original default_app_config suggestion,
except that

a) you've moved it to myapp.__init__ (which I definitely agree with)

b) you've changed the name to "required_app_config", which I think is a
worse name than "default_app_config". It's not actually required (the
user can explicitly specify a different one in INSTALLED_APPS if they
want), it's the default if no AppConfig is explicitly configured.

c) you've added an accelerated deprecation; I don't see how that is
justified. This isn't a security issue or otherwise an urgent change.

So I'm still +1 on the general approach with (a), but I think the name
should remain "default_app_config", and the deprecation, if we do one,
should wait until 1.8.

> [PS, I've opted for accelerated timings on this update path, which I
> justify on the basis that by using required_app_config we are in a sense
> changing the configuration of an app that the user has in their
> settings, therefore a visible warning is justified. However, I'm +0 on
> doing this over an extra release cycle]

I don't think that "in a sense changing the configuration of an app" is
different from any other change to the code of an app. Whether the
change deserves a visible deprecation warning depends on the urgency of
the user making some change, and there is no urgency here. I see no
justification for accelerating deprecation.

Carl

[Aymeric Augustin]

An interesting related problem hasn’t been raised yet. Since Django 1.7 is much more strict about the import sequence, apps that do stuff at import time will most likely break with RuntimeError("App registry isn't ready yet.”). They will have no choice but to run their initialization in AppConfig.ready().

Example: https://github.com/django-debug-toolbar/django-debug-toolbar/commit/37fadc0a0af4db7a0d3dd750e6a43efe4aed5ad9

If we keep the current implementation, users of such apps will be forced to change their INSTALLED_APPS without warning. It will drive usage of AppConfig with a stick. I’d rather do it with a carrot.

If we implement default_app_config and deprecate it immediately, the upgrade path becomes smoother. Users can run their applications first and fix their settings later, based on the warnings they get. They will have to do it by Django 1.9, anyway.

Given that the end result in Django 1.9 will the same, that app-loading will be quite disruptive in any case — most projets won’t even start after upgrading to 1.7, and that default_app_config is trivial to implement, I think it’s worth smoothing the upgrade path.

--
Aymeric.

[Russell Keith-Magee]

On Mon, Jan 20, 2014 at 11:09 PM, Jannis Leidel <lei...@gmail.com> wrote:

On 19.01.2014, at 08:56, Russell Keith-Magee <rus...@keith-magee.com> wrote:

> Hi all,
>
> First off - this isn't critical for 1.7 alpha - I'm raising it now because I've been in this space for the last couple of days as a result of the working on the check framework. I suspect any changes stemming from this discussion can be landed in the beta period without major problems.
>
> Also - Aymeric - you've done a great job with this app loading work. I didn't get much of a chance to follow along as it was unfolding, but the resultant work has been a pleasure to work with.
>
> With one exception :-)
>
> We've now got AppConfig objects for each app (e.g., contrib.admin). Those AppConfig objects can have ready() methods that invoke startup logic. This is all great stuff that has been a long time coming.
>
> *But*
>
> From a backwards compatibility perspective, there seems to be a gap in the way AppConfig objects are instantiated.
>
> The gap is this - historical apps (and historical documentation) calls for admin to be put into INSTALLED_APPS using 'django.contrib.admin'. However, when you do this, you get the system default AppConfig, not something that is admin specific. In order to get the new AdminConfig, you need to update your INSTALLED_APPS to point at "django.contrib.admin.apps.AdminConfig".
>
> 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.
>

Hi Russ, all,

Thanks for raising this issue as it’s indeed an important one to discuss. I hoped we’d have time to let the community figure out some of the techniques to handle version compatibility to Django but alas I didn’t expect the checks framework to so thoroughly crash the party. Well done! :) I wish we would have had more time to take these points into account when we were discussing the various details back in December, but I guess as long as 1.7 isn’t out it’s not too late. I probably would have made the argument that the check system may be best placed in AppConfig classes (e.g. in AppConfig.check_* methods). But on the other hand that may have made the work on the app config classes even harder.

We aren't entirely locked out on this as an option -- at the moment, we just have to register checks, and we could register a system level check that does a poll of all the apps looking for AppConfigs, and calling check on them. However, this is predicated on the relevant apps having an AppConfig, and being able to guarantee that this AppConfig is actually in use.

 

For the record, the app config concept has a long history with different implementations and goals but always (AFAIR) tried to fix one thing: the side effects of the implicit Django app concept. This issue is just another example of that problem. It bubbles up in our compatibility strategy which is to still allow dotted app module paths in INSTALLED_APPS. We assumed users to upgrade to the dotted app config paths in INSTALLED_APPS on their own eventually if the app in question required it. In other words, we really left it to the app authors to decide when to ask their users to make the jump. Instead of a hard coded check we wanted to gently push the new APIs into the community — also to find concept holes.

I now believe that we may have been too conservative since that strategy introduces a new level of uncertainty in the app API. I can see the user questions already: “Does the 3rd party app XYZ work on 1.7?” “Can I also install it on my legacy 1.6 project?” “How am I going to override some options without a AppConfig?” “What’s the best way to create cross version compatible apps?”

 

Agreed that this is a problem.

 

> I've dug through the mailing lists discussions about app-loading, and I can't see anywhere that this was explicitly discussed, so I can't tell if this was done deliberately, or as a side effect of the circular import problems that Aymeric described.
>
> 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 :-)

+1 It’s the strictest and but also clearest migration path and I believe we can’t afford to fix this over time and leave our users in limbo about how Django apps work or what to expect from the app API. It means it’ll be harder for reusable app developers to explain backward compatibility to their users but we could offer some pointers, e.g. in blog/models.py::

    try:
        from django import apps
    except ImportError:
        from blog.utils import register_something
        register_something()

That would work for Django versions < 1.7 and implies blog.utils.register_something is called in the ready() method of blog.apps.BlogConfig, too. It’s not an exciting pattern but at least common enough for Python developers to require little explanation.

You could also be explicit, and check "if django.VERSION < (1,7)".

An additional problem with this approach is how to actually evaluate this check. It's easy enough with contrib apps, because we know they are updated at the same time as Django, and we know they have been updated to use AppConfig. But how do we know if a third party app has been updated? AFAICT, the only way to know for sure is to probe for the apps module, which I thought was something we were trying to avoid.

 

> 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.

Yeah, this is a bad idea. I love conventions, but not if they prevent me to paint my shed blue. -1

> 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.

While I understand that it would allow us to keep using dotted module paths in INSTALLED_APPS, it’s also a great way to stall the adoption of app config classes. To be clear: it *is* the goal to eventually have all apps (contrib and 3rd party) use app configs as the only location for startup code and app configuration. App config classes are intended to be user and not only app developer facing. We strive to encourage users to be app developers as soon as possible, e.g. we tell them how to write an app in the tutorial. So introducing a hidden configuration variable like ‘default_app_config’ makes me really uncomfortable with regard to the mixed message we’d be sending. We should be clear about what app is used. The “default app config” is the one that is configured in the settings.py, just like the “default database backend” is the one defined there. In other words it’s the users choice what is considered default, not ours or the app developer’s.

As Marc indicated in his post, I don't think we should be treating this as a permanent feature of the API, but as but as a migration aid. Yes, default_app_config will exist as a bad smell for 2 releases, but come Django 1.9, we can remove it - and/or we can raise a hard InvalidConfiguration exception. 

We're then left with the following scenarios:

 1) Contrib apps, all updated to use AppConfigs in 1.7, and actually using them regardless of end user configuration as a result of default_app_config. Based on Marc's pull request [1], anyone actually relying on default_app_config gets a PendingDeprecationWarning, which accelerates until 1.9, when we can actually raise InvalidConfiguration.

 2) Third party apps who don't update anything. They continue to work as is, getting a default AppConfig object. If we're genuine about deprecating this usage, we should start raising a PendingDeprecationWarning whenever this happens, too (current code doesn't do this, so it isn't clear that migrating to "AppConfig all the time" is part of the plan).

 3) Third party apps that *do* update. They can use default_app_config to make sure existing usage continues to work, and any import-time registration hacks can be migrated to ready(). However, for backwards compatibility with older versions of Django, they maintain the import-time hack, but guard it with a "if django version < 1.7" clause.

To me, that seems to cover all our use cases, is backwards compatible to the extent possible, lets us clean up implicit registration logic whenever we can, and we only have to live with a bad smell for a couple of releases. While I agree default_app_config isn't ideal, I think that's a price worth paying.

[1] https://github.com/django/django/pull/2192

Yours,

Russ Magee %-)

[Carl Meyer]

On 01/21/2014 06:48 AM, Russell Keith-Magee wrote:
> As Marc indicated in his post, I don't think we should be treating this
> as a permanent feature of the API, but as but as a migration aid. Yes,
> default_app_config will exist as a bad smell for 2 releases, but come
> Django 1.9, we can remove it - and/or we can raise a hard
> InvalidConfiguration exception.

I've re-read Marc's post, and I still don't understand _why_
default_app_config is a bad smell, or why it should be deprecated so
eagerly. There have been a lot of assertions made that it is bad, but I
have seen very little by way of explanation of exactly what makes it bad.

I think it's an entirely reasonable and explicit convenience feature of
the API, in the long-term. For the majority of cases, users will want to
use the default AppConfig defined by the app author: what is the
concrete benefit of making them always type 'myapp.apps.AppConfig' into
INSTALLED_APPS when we could have a simple and explicit protocol in
place (documentable in no more than two sentences) that allows them to
use 'myapp' instead for the common case?

To me, rapid deprecation of default_app_config smells of forced churn
for the sake of conceptual purity without real benefit. I think it will
be much more confusing to users to get immediate deprecation warnings
about a new feature than it would be to continue allowing either 'myapp'
or 'myapp.apps.AppConfig' (presuming the app author has set
default_app_config).

At the moment, everyone's assertions about what default_app_config will
or won't do is based on speculation. If we add it with immediate
deprecation, we are forcing our hand in advance, without having any
actual data from experience. Why not add it for one release without
deprecation, so we can find out whether it is actually problematic in
practice? Then we still have the option of deprecating it if it is a
problem, but we haven't forced ourselves into a corner.

Carl

[Shai Berger]

On Tuesday 21 January 2014 20:03:25 Carl Meyer wrote:
> On 01/21/2014 06:48 AM, Russell Keith-Magee wrote:
> > As Marc indicated in his post, I don't think we should be treating this
> > as a permanent feature of the API, but as but as a migration aid. Yes,
> > default_app_config will exist as a bad smell for 2 releases, but come
> > Django 1.9, we can remove it - and/or we can raise a hard
> > InvalidConfiguration exception.
>
> I've re-read Marc's post, and I still don't understand _why_
> default_app_config is a bad smell, or why it should be deprecated so
> eagerly. There have been a lot of assertions made that it is bad, but I
> have seen very little by way of explanation of exactly what makes it bad.
>
> I think it's an entirely reasonable and explicit convenience feature of
> the API, in the long-term. For the majority of cases, users will want to
> use the default AppConfig defined by the app author: what is the
> concrete benefit of making them always type 'myapp.apps.AppConfig' into
> INSTALLED_APPS when we could have a simple and explicit protocol in
> place (documentable in no more than two sentences) that allows them to
> use 'myapp' instead for the common case?
>

+1 (actually, +1 for most everything Carl said in this discussion).

Shai.

[Russell Keith-Magee]

For my part, it's a light smell, in that it's a configuration variable that's in a place you wouldn't historically expect to find a configuration variable, for the purpose of defining a string substitution, which makes it mildly implicit behavior. Based on explicit > implicit, I'd rather see users actually type what they want to get (an AppConfig).

That said, I also agree with what you've said here. There's merit in keeping the user experience simple and consistent with the existing usage, and rapid deprecation churn does concern me a little. I'm happy to support making default_app_config an immediately depreacted migration aid if it makes the medicine more palatable to those opposed to it, but I'd be just as happy if we kept it as a permanent API (or at least punt the decision to a later release). 

 

Russ %-)

[Aymeric Augustin]

I just pushed a patch based on this discussion:
https://github.com/django/django/commit/2ff93e027c7b35378cc450a926bc2e4a446cacf0

On 22 janv. 2014, at 00:08, Russell Keith-Magee <rus...@keith-magee.com> wrote:
> For my part, it's a light smell, in that it's a configuration variable that's in a place you wouldn't historically expect to find a configuration variable, for the purpose of defining a string substitution, which makes it mildly implicit behavior. Based on explicit > implicit, I'd rather see users actually type what they want to get (an AppConfig).

This is tracked here:
https://code.djangoproject.com/ticket/21822

--
Aymeric.