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

-- 
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/CAJxq849Hx24ETnugK54xGGRpQNUFON18YSwPdXKv0fYg%2B7tnxQ%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Feature request: post/pre commit/rollback signals

[Carl Meyer]

On 01/18/2014 01:19 PM, Anssi Kääriäinen wrote:
> On Saturday, January 18, 2014 9:02:41 PM UTC+2, Aymeric Augustin wrote:
> 
> On 18 janv. 2014, at 19:50, Carl Meyer  > wrote:
> 
> > Those interested in this feature may find django-transaction-hooks
> [1] useful. It is my attempt to implement the commonly-useful case
> (run some code after the current transaction successfully commits,
> providing the code wasn't registered during a savepoint which was
> later rolled back.) I think the API provided by
> django-transaction-hooks is actually better for that use case than
> transaction signals are, since it does the work of handling some
> situations correctly (savepoints, closed connections), which in the
> case of signals are left up to the signal receiver to handle (or
> more likely, not bother to handle.)
> >
> > Testing and feedback welcome. If this becomes widely used and
> seems to fill a need, I think there is a chance it could become part
> of Django core. (See #21803.)
> 
> I think this is the right approach. I would use that package. I
> support adding it to Django.
> 
> 
> If documentation warns clearly that there is no guarantee whatsoever
> that the hook is always ran after commit +1 for addition in Django. The
> most obvious case of abuse for this sort of hook is doing something
> business critical in the post-commit hook. It must be clear that this is
> not a substitute for two phase commit or other ways of avoiding
> crash-just-after-commit problem.

Right. I have this warning in the current docs for
django-transaction-hooks, though it should probably be more prominent.
I'll make sure it gets into the eventual Django patch, too.

Carl



signature.asc
Description: OpenPGP digital signature

Re: Feature request: post/pre commit/rollback signals

[Anssi Kääriäinen]

On Saturday, January 18, 2014 9:02:41 PM UTC+2, Aymeric Augustin wrote:
>
> On 18 janv. 2014, at 19:50, Carl Meyer  
> wrote: 
>
> > Those interested in this feature may find django-transaction-hooks [1] 
> useful. It is my attempt to implement the commonly-useful case (run some 
> code after the current transaction successfully commits, providing the code 
> wasn't registered during a savepoint which was later rolled back.) I think 
> the API provided by django-transaction-hooks is actually better for that 
> use case than transaction signals are, since it does the work of handling 
> some situations correctly (savepoints, closed connections), which in the 
> case of signals are left up to the signal receiver to handle (or more 
> likely, not bother to handle.) 
> > 
> > Testing and feedback welcome. If this becomes widely used and seems to 
> fill a need, I think there is a chance it could become part of Django core. 
> (See #21803.) 
>
> I think this is the right approach. I would use that package. I support 
> adding it to Django. 
>

If documentation warns clearly that there is no guarantee whatsoever that 
the hook is always ran after commit +1 for addition in Django. The most 
obvious case of abuse for this sort of hook is doing something business 
critical in the post-commit hook. It must be clear that this is not a 
substitute for two phase commit or other ways of avoiding 
crash-just-after-commit problem.

 - Anssi

-- 
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/574600c8-2aba-4ac6-bb19-f28e0a13a46d%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Feature request: post/pre commit/rollback signals

[Andrey Antukh]

Hi!

2014/1/18 Aymeric Augustin 

> On 18 janv. 2014, at 19:50, Carl Meyer  wrote:
>
> > Those interested in this feature may find django-transaction-hooks [1]
> useful. It is my attempt to implement the commonly-useful case (run some
> code after the current transaction successfully commits, providing the code
> wasn't registered during a savepoint which was later rolled back.) I think
> the API provided by django-transaction-hooks is actually better for that
> use case than transaction signals are, since it does the work of handling
> some situations correctly (savepoints, closed connections), which in the
> case of signals are left up to the signal receiver to handle (or more
> likely, not bother to handle.)
> >
> > Testing and feedback welcome. If this becomes widely used and seems to
> fill a need, I think there is a chance it could become part of Django core.
> (See #21803.)
>
> I think this is the right approach. I would use that package. I support
> adding it to Django.
>

You are absolutely right! This is much best approach than using signals (I
also don't like signals, my approach was only for show a use case).
And I think that this is much less intrusive (and without signals), and
would be awesome if it be added to django. This use case is very common in
web development.


> Since it doesn’t do anything until after the transaction has successfully
> committed, it cannot interfere with the transaction management code. That’s
> my most important requirement and the reason why I’m so strongly opposed to
> pre-commit/rollback signals.
>
> If you look at the first email in this thread, the use case is “do
> something when a transaction is committed successfully”. In the later email
> that gives an example, it only uses post_commit. Generic transaction
> signals look like a collective hallucination to me, or at least a pretty
> bad case of YAGNI.
>
> --
> Aymeric.


Thanks!

Greetings.
Andrey


-- 
Andrey Antukh - Андрей Антух -  / 
http://www.niwi.be 
https://github.com/niwibe

-- 
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/CAKn%3DmONTxxVA_HMHuMzxabL6zgG83f1NpUQyaoqV13LY5y%3DZ1A%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Feature request: post/pre commit/rollback signals

[Aymeric Augustin]

On 18 janv. 2014, at 19:50, Carl Meyer  wrote:

> Those interested in this feature may find django-transaction-hooks [1] 
> useful. It is my attempt to implement the commonly-useful case (run some code 
> after the current transaction successfully commits, providing the code wasn't 
> registered during a savepoint which was later rolled back.) I think the API 
> provided by django-transaction-hooks is actually better for that use case 
> than transaction signals are, since it does the work of handling some 
> situations correctly (savepoints, closed connections), which in the case of 
> signals are left up to the signal receiver to handle (or more likely, not 
> bother to handle.)
> 
> Testing and feedback welcome. If this becomes widely used and seems to fill a 
> need, I think there is a chance it could become part of Django core. (See 
> #21803.)

I think this is the right approach. I would use that package. I support adding 
it to Django.

Since it doesn’t do anything until after the transaction has successfully 
committed, it cannot interfere with the transaction management code. That’s my 
most important requirement and the reason why I’m so strongly opposed to 
pre-commit/rollback signals.

If you look at the first email in this thread, the use case is “do something 
when a transaction is committed successfully”. In the later email that gives an 
example, it only uses post_commit. Generic transaction signals look like a 
collective hallucination to me, or at least a pretty bad case of YAGNI.

-- 
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/A8A2614C-FF80-4978-9DE6-B85A41C9FE99%40polytechnique.org.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Feature request: post/pre commit/rollback signals

[Carl Meyer]

Hi,

Those interested in this feature may find django-transaction-hooks [1] 
useful. It is my attempt to implement the commonly-useful case (run some 
code after the current transaction successfully commits, providing the code 
wasn't registered during a savepoint which was later rolled back.) I think 
the API provided by django-transaction-hooks is actually better for that 
use case than transaction signals are, since it does the work of handling 
some situations correctly (savepoints, closed connections), which in the 
case of signals are left up to the signal receiver to handle (or more 
likely, not bother to handle.)

Testing and feedback welcome. If this becomes widely used and seems to fill 
a need, I think there is a chance it could become part of Django core. (See 
#21803.)

Carl

  [1] https://github.com/carljm/django-transaction-hooks

-- 
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/200b56e4-6577-451f-889d-d50678e77496%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Merging GSoC 2013 - Validation Refactor

[Russell Keith-Magee]

Hi all,

After a long delay, I'm finally ready to merge the result of Christopher
Medrela's 2013 Summer Of Code Project - a refactor of Django's validate
management command.

A huge thanks to Christopher for his excellent work over the summer - he
made my job as mentor very easy.

For those that didn't follow along with the project itself - Django's
validate management command was a single monolithic block of code that
checked for name collisions, certain allowable attribute values, and so on.
It wasn't extensible in any way.

The purpose of this refactor was to break this monolithic method up into
smaller parts, and along the way, provide a way for other apps/libraries to
hook in and provide their own checks. So, for example, GenericForeignKeys
now get validation support - this wasn't previously possible because they
are part of a contrib app, and therefore couldn't be referenced in the core
codebase.

So - if you've got a field, model base class or model manager that needs
custom validation that you'd like to be reported in the same way as
./manage.py validate has historically reported, you now have a way to do so.

Or, if you've got some other system check that you'd like to perform (for
example, performing a security audit), you can hook in to the same tools.

As part of this refactor, we've also dealt with the manifold overloading of
the term "validate" in Django's codebase. We've deprecated "validate" in
preference for the "check" command.

A side effect of this is that the version update checks introduced in 1.6
are now guaranteed to run every time you do runserver. To make sure you
aren't needlessly bothered once you've dealt with these issues, individual
message types can be silenced. So, for example, when you do a version
update, you may be warned about a change; once you're satisfied that you've
met that requirement, you can silence that error and never see it again.

We also allow for different severity levels: DEBUG, INFO, WARNING, ERROR,
CRITICAL. This provides a lot more options for reporting problems that
aren't show-stoppers, but probably warrant attention (e.g., easily
identifiable performance issues).

I've created a clean PR based on a rebased version of the code that has
been brought up to date with trunk:

https://github.com/django/django/pull/2181

There are a still a couple of minor issues that may need to be addressed -
in particular, I want to do a cleanup of the language for the messages
themselves, and the documentation would probably benefit from another pair
of eyes. However, I'm happy with the way the core has shaken out; I want to
get it into core so it gets more exposure.

Tim's review a couple of days ago also pointed out a couple of interesting
features (in particular, adding a command-line flag to silence specific
warnings) which might be nice to have, but shouldn't affect core
functionality.

Barring objections, I'm planning to land this in time for the 1.7 alpha (so
Andrew - no cutting releases until I've had a chance to push the merge
button :-)

So - final chance for comments and feedback before this lands!

Yours,
Russ Magee %-)

-- 
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/CAJxq84_h2RAVcSOCxthcd%3DEf4K2P5NqoJs4zc%2BLirBE744McPw%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.