On Tue, Jun 1, 2010 at 3:14 PM, Vinay Sajip <vinay_sa...@yahoo.co.uk> wrote: > > 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.
IMHO, that use case would be better handled with two different settings files -- one with DEBUG=True and debug logging, and one with DEBUG=False and production logging. Having multiple settings files isn't an unusual deployment arrangement; there are many guides to Django deployment that will advise that approach. Your proposal also requires that we start recommending that users put logic into their settings.py, and I'm not comfortable making that a recommended practice. Nothing else in a Django settings file is a callable; I don't see why logging needs to break that trend. My YAGNI objection is grounded in the fact that I can't see why lots of flexibility is required in the logging configuration process. For all the example cases I can envisage, there is exactly 1 function call that the logging setup process needs to make, and the value passed to that function is a configuration dictionary that can live in settings. *If* we were to accept the use case that configurable logging setup function is required, I'd be much more inclined to have a LOGGING_SETUP = 'path.to.my.setup_function', with a default value that points to an implementation that just calls configure_logging_from_dict(). However, we can also add this later in an entirely backwards compatible way if it turns out to be a common requirement. > 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)." This is exactly why startup.py is being proposed. We need to have a place to reliably register signal handlers, search callbacks, and anything else that has historically been put in urls.py. > 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. "No extra cost" isn't a very good argument, because there is always a cost. In this case, it's not the cost of implementation or execution that concerns me - it's the cost of having a public API that we need to maintain, and a point of backwards compatibility that needs to be maintained if we ever modify the model loading infrastructure (something that the GSoC is working on, for example). >> 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. It's not about the names, it's about the pattern that is being reimplemented. Your proposal may well be simple, but it doesn't change the fact that "callbacks" are essentially a reimplementation of exactly what signals do, but with less flexibility. A signal is just an agreement to call a bunch of functions -- exactly like the callback you describe, except that signals allow for a generic registration (rather than using a setting), and signals can be filtered based on the arguments with which the callback was invoked. As your quote from Simon indicates, there is a need to provide a reliable location that can be used for registrations, installing signal handlers, and other 'on startup' behaviors. This can't be a signal because you need to have somewhere to install the first signal handler. So, startup.py needs to use a different approach, and needs to happen as early as possible in the process. Right after logging is configured is as good a place as any. The only operational difference between startup.py and your PRE_MODEL_CALLBACKS list is that startup.py doesn't require any new settings in order to work -- you just drop a startup.py in your app, and you're done. However, once this initial startup has been invoked, any other callback that is required can be handled by signals. There is no operational difference between the POST_MODEL_CALLBACKS and a 'models_loaded' signal; but again, the signal approach doesn't require any new settings. All we are arguing about is the need for this signal at all. Your comment about avoiding the model loading race condition is certainly a valid one, and that's probably enough to get me over the line on the need for this signal. My only concern is whether there is anything else that a "models_loaded" signal should be 'after'. If we're going to introduce a "we've finished setup" signal, I want to make sure that the place we emit it is a point at which we can guarantee that we've actually finished the setup :-) Yours, Russ Magee %-) -- 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.
