Hey Aymeric, thanks a lot for your immediate feedback. I appreciate that a lot. I've added a few comments inline.
On 30/06/14 23:55, Aymeric Augustin wrote: > 2014-06-30 14:32 GMT+02:00 Sebastian Vetter > <sebast...@roadside-developer.com > <mailto:sebast...@roadside-developer.com>>: > > 1) Creating a subclass of AppConfig for a Django app is (according to > the docs) not required. If none is defined, Django falls back to > AppConfig. > > For a pluggable app, is it recommended to always subclass AppConfig and > provide default_app_config? > > > There are two reasons for doing that: > > 1) Provide a translatable verbose_name for the admin. If your application > doesn't have models, or if it isn't translated and the capitalised app_label > is what you need, this reason doesn't apply. > > 2) Run code at start-up. If your application does this, I would encourage > you to move it inside AppConfig.ready(), even if it works just fine at > module-level in models.py. > > If neither matters for your application, don't bother with > default_app_config, > just use Django's default AppConfig. That makes sense. I gues in practise this means defining a AppConfig most of the time if just for reason 1). > 2) The recommended way to register signals with receivers now (assuming > they should be registered automatically) is the ready() method. > Previously, the only "safe" way to ensure they would be connected was > doing that in the 'models.py'. > > For a pluggable app this can be tricky if you are trying to maintain > backward-compatability. The only way I can think of is checking in the > 'models.py' for a Django version < 1.7 and only run the code then. Is > there another/better way to do that? > > > That's the pattern I'm advocating. One may find explicit version checks > inelegant, but in practice they just work. For an example, see: > https://github.com/django-debug-toolbar/django-debug-toolbar/blob/master/debug_toolbar/models.py Inelegant is the right word :) But I agree that it works and with a deprecation on top of it they might not have to stick around for too long. > 3) Across several pluggable apps that I've worked on, there have been > several occasions where a model from one app is imported into the > 'models.py' of another app. Now this causes issues with the new > AppConfig if I call 'get_model' at the module level. The only way I see > around it is to pull 'get_model' into that function(s) where this model > is used...which seems bad. An example would be here [1]. It might not be > the best example but one that I've come across just a few days ago. > > Is there a better way to deal with this when using the AppConfig? Or > should that be avoided in general to import model in another apps' > 'models.py'. > > > You can import models from other apps explicitly: > from otherapp.models import SomeModel > > What doesn't work is to obtain them through get_model(). As far as I remember, the explicit import of models was discourage in favour of get_model(). Is that no longer the case with the new app registry? Do you see portential problems with explicit import and the app registry initialisation? > This is often an issue with get_user_model(), which is just a shortcut for > get_model(settings.AUTH_USER_MODEL). > > At this point, my best advice is to use string references eg. 'app.Model' > at import time and resolve them at runtime. That's how Django's custom > user models are implemented. > > If you're asking with django-oscar in mind, I'm following closely the story > there. My team is currently building an Oscar shop. We're going live in > September and we'd like to do so on Django 1.7. I'll be following that conversation closely. A few of the apps that I'm maintaining/working on around Oscar are using similar patterns for obvious reasons and the challenges/solutions are most likely to be the same or similar. > 4) A project that I am working with is django-oscar [2] which defines an > 'app.py' which contains a subclass of 'Application' (e.g. [3]). The main > purpose is to provide an overridable way to define URL pattners by > providing them through a 'get_urls' method. > > It seems intuitive to me to consolidate functionality like that into the > AppConfig subclass for an app. Are there any reasons for or against > doing this? Are there any implications this might have on the loading of > the app registry? > > > I see this as a variant of the common suggestion of managing app-specific > settings in AppConfig classes. We might want to implement that at some > point. But I'd like to see how the dust settles on my refactoring first. > > In the mean time, nothing prevents third-party apps from providing mixins > implementing that feature. That makes absolute sense. I mainly wanted to check that there isn't anything in there or in the works that might make this difficult. > Regarding django-oscar, I have a hunch that its Application class should > subclass AppConfig on Django 1.7. But that would make it difficult to > support both 1.6 and 1.7 in the same release. I think your mixin suggestion from above would still be possible. It's something that I was planning to suggest on the mailinglist over there. Thanks again, Sebastian
signature.asc
Description: OpenPGP digital signature
