Code review comment for lp:~le-chi-thu/lava-dispatcher/improve-logging

Revision history for this message
Le Chi Thu (le-chi-thu) wrote :

>8 +./.idea
>What's this for?

.idea is created by the IDE (PyCharm) I use containing PyCharm project
files..

> You should be able to just use get_config() for this (see
> lava_dispatcher/config.py)
>
> I saw and understand the get_config method in config.py but It does not fit
the need of logging.

I need to pass the url of the configuration file to the python logging
library. So I use the load_config_paths to search for that file.

get_config get keys and values inside the configuration file.

477 +[formatters]
> 478 +keys: detailed,simple
>
> 502 +handlers: console
> Perhaps I'm just tired and missing it, but I don't see where you are
> actually using all these options. Also, I think it's overkill. I don't
> think we should really need to specify anything other than the log level,
> and I'm thinking perhaps it should be set to some default value in the main
> dispatcher config file, but be something we can override with an option
> passed to the job.
>

This is how python logging configuration commonl done. This file is a simple
template and can be override by copy the file to users lava-dispatcher
configuration directories and modify the content.

See example in http://antonym.org/2005/03/a-real-python-logging-example.html

> Otherwise, looking much better!
> --
>
> https://code.launchpad.net/~le-chi-thu/lava-dispatcher/improve-logging/+merge/71120
> You are the owner of lp:~le-chi-thu/lava-dispatcher/improve-logging.
>

« Back to merge proposal