Patch for letting multiple engines and pools use a single stdlib logger each

[Former user]

As per our discussions on Reddit and sqlalchemy-devel.

[Former user]

Patch to allow engines and pools to share stdlib loggers

[Mike Bayer]

this looks really nice. I'm going to tag this for 0.7 initially. When I get a chance to play with it, I may consider moving it into 0.6. thanks for the effort !

[Mike Bayer]

mmm unfortunately this patch doesn't really do what I need at all. I'm working with it and considering various simplifications for 0.7 - but the basic contract of the "echo" flag, and the reason its so diffcult, is this:

e = create_engine('sqlite://', echo=True)
e.execute("select 1")

and the logging works. Your patch removes the call to default_logging(), so the above script fails. Unusually, it actually dies with the message 'No handlers could be found for logger "sqlalchemy.engine.base.Engine"', and no stack trace or anything ! not sure what's causing that yet, that seems exceedingly unusual that there would be no stack trace.

[Mike Bayer]

I have a modified version of the patch forthcoming that solves the abovementioned issue. I now see that your patch relies upon calling the publically undocumented logger._log(). Is this method safe to use? I.e. good in Python 3K, etc. ? should Python logging get a new method "log_internal()" or something like that ?

[Former user]

Replying to zzzeek:

and the logging works. Your patch removes the call to default_logging(), so the above script fails. Unusually, it actually dies with the message 'No handlers could be found for logger "sqlalchemy.engine.base.Engine"', and no stack trace or anything ! not sure what's causing that yet, that seems exceedingly unusual that there would be no stack trace.

I missed the default_logging() call - an oversight, as I was concentrating on mapping echo values to logging levels while keeping just a single logger. No script should actually die after printing that message, though no logging will occur because no handlers were available: it's a one-off message to guard against accidental misconfiguration, and is only printed once. As there's no exception raised, there's no stack trace.

Ideally libraries should not add handlers automatically - usually it should be up to the application developer to configure handlers - and it's not ideal that setting the echo flag both sets verbosity and adds a handler - though you may be constrained from changing anything here because of backward compatibility.

[Former user]

Replying to zzzeek:

I have a modified version of the patch forthcoming that solves the abovementioned issue. I now see that your patch relies upon calling the publically undocumented logger._log(). Is this method safe to use? I.e. good in Python 3K, etc. ? should Python logging get a new method "log_internal()" or something like that ?

It should be safe to use {{{_log()}}}. It's not officially documented because I don't think most users will need to know about it or override it.

It's only in this patch because my original implementation of {{{LoggerAdapter}}} goofed in the reusability department; I couldn't usefully subclass it to deal with this use case, even though it was meant for this sort of scenario :-(

So I made a new implementation of {{{LoggerAdapter}}} which will go into Python3.2, and as I had no choice, I copied it into the patch as {{{InstanceLogger}}}. If I hadn't goofed originally, {{{InstanceLogger}}} would just subclass {{{LoggerAdapter}}} and override {{{getEffectiveLevel()}}}. (At some future point where you drop support for Pythons older than 3.2, you can change the implementation to do just this, and the {{{_log()}}} call will disappear from SQLAlchemy's codebase.)

The improved {{{LoggerAdapter}}} is also available in an external {{{logutils}}} project which brings recent logging features to older Python versions. This of course uses {{{_log()}}} in the same way; so I don't intend changing {{{_log()}}} in any way which causes existing code to break.

Since the original release of logging, the only changes to {{{_log()}}} have been the addition of the optional ''extra'' keyword parameter, and very recently (for 3.2) the optional ''stack_info'' keyword parameter.

[Mike Bayer]

agree on libraries adding handlers. but the echo=True is unbelievably useful for quick scripts. if the echo flag isn't used, none of the handler stuff is used either.

[Former user]

Replying to zzzeek:

agree on libraries adding handlers. but the echo=True is unbelievably useful for quick scripts. if the echo flag isn't used, none of the handler stuff is used either.

I see that. What I say about libraries adding handlers is that they shouldn't do so ''automatically'', but that it's OK for them to provide a documented convenience API that adds handlers. The echo flag is effectively that, and as long as application developers are made aware that setting echo has the double effect of setting verbosity + adding handlers, there (hopefully) shouldn't be any problems.

[Mike Bayer]

diff:@2336b1cebfcb2f304e09cbc2a0e8bb3fb3a9ceeb:ccf77190f986095834220ece160992e224dcc587

[Mike Bayer]

Removing milestone: 0.7.0 (automated comment)