Logging re-initialized in make_app()

[Mads Kiilerich]

On 12/4/19 1:36 AM, Wolfgang Scherer wrote:
> When following the instructions for WSGI dispatch at https://kallithea.readthedocs.io/en/latest/setup.html#apache-with-mod-wsgi any attempt to pass defaults to the logging initialization fails later, when logging is initialized again in :func:`make_app` of |kallithea/config/middleware.py|.
>
> See http://sw-amt.ws/kallithea-deploy/html/overview.html#bug-logging-re-initialized-in-make-app for a full description of the bug.
>
> Attached is a patch, that works for me.


Thanks. That's a lot of text. Please help clarify what are the main 
points and what are additional information / supportive evidence?

First, what do you mean with "pass defaults to the logging 
initialization"? Exactly how do you do that?

When you say "fails later" - exactly how does it fail?


I do agree this area looks very dirty. So much is happening in all the 
middle layers, all trying to make it simple, but ending up making it 
much more complex.

Python logging is generally not proper namespaced. Multiple attempts at 
controlling logging doesn't merge well, and there must thus be one place 
that controls it. It is thus essentially like a global variable. As 
https://flask.palletsprojects.com/en/1.1.x/logging/ says "When you want 
to configure logging for your project, you should do it as soon as 
possible when the program starts". For kallithea-cli we do that (even if 
perhaps not entirely correctly). For use as WSGI application, it gets 
more complicated as applications can be combined in the same process. 
Logging must essentially be configured by the top level entry point ... 
which must be the WSGI server or the top level WSGI application. It is 
thus kind of wrong to initialize logging in the WSGI dispatch script ... 
but more correct than to do it in the Kallithea application. Yet, to 
support different setups, I guess we still have to do it in the 
application if it hasn't been done before.

I find it essential for understanding middleware-logging.patch that it 
is guarding the initialization similar to how logging.basicConfig does. 
This is thus kind of to establish our own basicConfig, for the same reasons.

Also, the patch is establishing a wrapper around fileConfig that provide 
the `__file__` and `here` values (which seems to be a convention in this 
stack). Do you agree that we should use this wrapper in *all* places, 
instead of using fileConfig directly? If so, I will rework the patch to 
do that and clarify the back story in the commit message.


/Mads