the optionally cross dependent app 'problem'

[Dougn]

As pinax grows more and more apps are becoming cross integrated. Most
try to do this in an optional way.
To be specific django-messages 'optionally' uses django-notifications.

There are issues with how this 'optional' part works. The current
implementation has a nasty bug:

across code like friends, swaps, games, etc, we see code like:
try:
from notification import models as notification
except ImportError:
notification = None

The problem with this code is that notification will be loaded (and
used) even if notification is not in INSTALLED_APPS. The fact that
notification is on the python path does not mean that it is available.

In discussions in IRC we came up with 3 different solutions, each with
their own problems/benefits

As this issue effects many of the pinax application projects it needs
to be opened to a wider discussion.
Clearly we need a simple, and consistent way to deal with this type
of integration.
So lets review the 3 that were come up with (if someone has a 4th or
5th better option, then lets hear them).

1. verbose and explicit
===========================
from django.conf import settings
try:
from notification import models as notification
if not 'notification' in settings.INSTALLED_APPS:
# if the user hasn't added notification to INSTALLED_APPS he
# might have it on PYTHONPATH but do not want to use it.
raise ImportError
except ImportError:
notification = None

Benefits:
------------
1.b.1. clear, explicit, does what we want?

Problems:
--------------
1.p.1. a bit verbose

1.p.2. can mask import errors in notifications
If there is an import error (due to say a feature being removed from
django or a typo or a dependency in notification being missing) then
it could be masked by this code depending on the ordering in
INSTALLED_APPS.
This is because python will create an empty module called
'notification.models' in sys.modules, so the next time someone tries
to from notifications import models, it will get that empty module,
and no import error. If the app with the above code is on the
INSTALLED_APPS list before notification, then you could get django to
'validate' but the actual app functionality would be broken with
errors like 'module notification has not attribute send'.

I fear that is a deal breaker.

1.p.3. assumes you always want to integrate if present.


2. Use django's app loader
======================
from django.db.models import get_app

try:
notification = get_app('notification')
except ImproperlyConfigured:
notification = None

Benefits:
--------------
2.b.1. very little code
2.b.2. ImportError's will still be raised

Problems:
---------------
2.p.1. get_app is not a documented feature?
I doubt this interface will change or be removed anytime soon, but do
we want to recommend a common practice which uses an 'unsupported'
call? (granted this call and others from the same registry are used
all over django, and in 6 of the pinax apps already) This was a major
bone of contention in IRC.

2.p.2. Only apps with 'models.py' are returned by get_app
Due to what this call is actually used for in django, only those apps
with a 'models' module is supported, and its the models module which
is returned. Some apps (like mailer) have their integration at the top
app level.

2.p.3. get_app uses the app_label.
So to get django.contib.admin.models you would use get_app('admin').

2.p.4. assumes you always want to integrate if present.

3. Be explicit with your integrations
===========================
if getattr(settings, 'MESSAGES_USE_NOTIFICATION', False):
from notification import models as notification
else:
notification = None

Benefits:
-------------
3.b.1. small
3.b.2. wont mask ImportError
3.b.3. explicit
each app can be 'integrated' separately, so if you dont want messages
to use notification, but you want friends to use it, you have that
control.

Problems:
--------------
3.p.1. Default False? True?
if you say that it defaults to True, then there will be problems for
generic clients of the app. if you default to False, then will it ever
be used? Really it will turn it into a 'hidden' feature with little
exposure.

3.p.2. Not pure boilerplate
For the other examples the code could be cut&pasted as is for a single
module integration. Sure you have to change it for each different
'notification' type app there is, but that change is minimal. The
creation of explicit settings options means we are dictating to
reusable apps how to set up settings variables (is that a bad thing?)

3.p.2. settings namespace EXPLOSION
If every app/app pair did this we would have a massive n^2 boolean
settings. If these defaulted to True, it would be quite nasty for
anyone trying to use a reusable app standalone. If would be equally
bad for pinax which wants all these things integrated.



Cleaning up the meme
======================

One thing that I noticed was that the same app will use the
boilerplate in many locations throughout an app. This might be just
because notifications are handy to be sent from model signals, views,
forms, etc. and as such are integrated into all those files. I have
seen the mailer also brought in in a number of locations. Instead of
repeating all this code multiple times within the same app, how about
coming up with a standard module for dealing with this. something like
'dependencies.py' (dont care for the name but I am winging it here...)

So you would have something like:

frields/dependencies.py:
from django.db.models import get_app

try:
notification = get_app('notification')
except ImproperlyConfigured:
notification = None

try:
send_mail = get_app('mailer').send_mail
except ImproperlyConfigured:
from django.core.mail import send_mail

and then in your models.py, views.py, forms.py, etc:

from dependencies import notificaiton, send_mail

if notification:
notification.send(...)


thoughts?

-Doug