On Jun 1, 3:07 am, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
>
> My concern here is YAGNI. Can you suggest a specific use case where
> this will actually be required?
>
I agree with the YAGNI principle generally, but to my mind it's
something to bear in mind if one were implementing a new feature with
a fair amount of additional code. In this case, we're talking about a
choice between (using pseudo-names for the functions)
# in Django code after importing settings
configure_logging_from_dict(settings.LOGGING) # your proposal
versus
# in Django code after importing settings
settings_callback() # my proposal
# in settings.py
def settings_callback():
configure_logging_from_dict(LOGGING) # the normal case
which has the advantage that I can replace the "normal case" with any
specific stuff that I want. For example, pick between different
configurations depending on some other value in settings.py:
# in settings.py
def settings_callback():
if DEBUG:
logging_config = DEV_LOGGING
else:
logging_config = PROD_LOGGING
configure_logging_from_dict(logging_config)
I don't think this use case would be uncommon. Now I'm not saying you
couldn't do this any other way, but the above seems a reasonably
natural way of doing it.
> No - the startup files would be imported immediately after
> configuration has completed, but before any models have been loaded.
> This is probably closest to PRE_MODEL_CALLBACKS in your setup. This
> means you could connect to the class_prepared signal in startup.py.
>
Okay. I thought you were talking about POST_MODEL_CALLBACK because you
mentioned importing models in startup.py.
> There is a potential race condition here. A model import that occurs
> in startup.py will cause the model loading infrastructure for that
> model to be invoked. This means it would be possible to load a model
> before you get a chance to install the class_prepared signal handler.
> However, the same race condition exists in your own proposal. If you
> did a model import in a pre-model-callback, that would cause those
> models to be registered, so a pre-model callback can't actually
> guarantee that the models *haven't* been loaded.
But that can be easily covered by documentation. The point of the
having the two callback points is that you can add hooks before and
after model loading. It's easy enough to prominently warn in the
documentation that PRE_MODEL_CALLBACK functions should not expect
models to be available, and that "if you want to do that, put it in a
POST_MODEL_CALLBACK". The very prefixes PRE_MODEL_ and POST_MODEL_
give some indication about the availability of models. (By the way,
those names xxx_CALLBACKS are just placeholders, and if better names
could be found, then that would be great.)
With your proposal, the race condition is harder to avoid because
there's only one callback point. If I want, in a callback, to import a
model, I can't place it in a POST_MODEL_CALLBACK because there isn't
one.
> This is just one of the unfortunate eccentricities of Django's model
> loading and registration process. The only solution I can offer here
> is to say that the order in which startup.py's will be invoked will be
> predictable, so if you need to guaranteed that a class-prepared
> handler will be added, you can add an app to INSTALLED_APPS whose only
> purpose is to register a handler for class_prepared signals, or jiggle
> the order of INSTALLED_APPS to avoid this sort of side effect.
Well, isn't my proposal an alternative solution? What's wrong with
having PRE_MODEL_ and POST_MODEL callback points? Is it an aesthetic
objection? The example I gave initialises logging in the bootstrap
callback, hooks class_prepared in the pre_model callback and imports a
model in the post_model callback. From my perspective, it seems to
cover pretty much what most use cases would need (if that's not so,
please chime in with the not-covered use cases).
> The issue of adding other signals is orthogonal to the logging
> request, AFAICT. You could certainly make an argument for a
> "apps_loaded" signal that is emitted after all the INSTALLED_APPS have
> been loaded. However, I don't see that this is a specific requirement
> for logging, and as long as we provide an entry point early in the
Sure, but I believe that people have brought up the startup hook issue
before, not always in the context of logging. To quote Simon Willison
[1]:
"The problem is that Django's startup execution order is poorly
defined - stuff relating to settings is lazily evaluated the first
time it's needed, and there's no real documented place to put code
that needs to be executed once (the fact so many registration things
end up being called from urls.py is a good indicator that this is a
problem)."
The way I've set things up, init_logging is called only once, though
settings.py is imported twice.
Logging is just one instance of something you want to do early in the
startup process. The mechanism I've provided is more general than the
minimum needed for logging, and the only compelling reason I can see
for disallowing it is that you think it somehow allows people to shoot
themselves in the foot, or "it's just not the Django Way". The default
settings.py created by django-admin startproject could have the
init_logging callback defined as in my example, so that users don't
need to do anything more than set up settings.LOGGING according to
their needs. IOW, for many uses, the users have no extra work to do
between our two proposals. So, easy things are easy. But having the
more general mechanism also makes harder things possible at almost no
extra cost, and in my view that's a good thing.
> startup process, I don't see that this is something that requires a
> callback system independent of Django's existing signals framework.
> Again, if you can provide a specific use case to prove otherwise, I'm
> willing to be convinced.
It's hardly a "callback system independent of Django's existing
signals framework", by which I mean it's hardly worthy of calling it a
"system" as it's a lot simpler. Basically, just something like
if hasattr(settings, 'PRE_MODEL_CALLBACKS'):
callbacks = settings.PRE_MODEL_CALLBACKS
for callback in callbacks:
callback()
which is pretty darn simple, to my mind and hardly a competing
framework to the signals stuff. If you think the names would cause
confusion (e.g. BOOTSTRAP, CALLBACK, then those can easily be changed.
If your objections are on aesthetic/taste grounds, I completely
understand - that's a personal preference and there's no point in
trying to sway anyone from their personal style preferences. But in
terms of meeting the functional requirement (both logging
configuration and the more general "do stuff on start-up once and once
only") and bang for buck, what I've proposed is simpler, involves less
code than a separate infrastructure of startup.py files, and provides
for more advanced usages (e.g. importing models in a callback without
worrying about race conditions) without making the simple cases
harder.
Regards,
Vinay Sajip
[1] http://groups.google.com/group/django-developers/msg/19f1ba7e4614cff6
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
