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

Revision history for this message
Paul Larson (pwlars) wrote :

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

31 +# load logging configuration file
32 +for directory in load_config_paths('lava-dispatcher'):
33 + path = os.path.join(directory, 'logging.conf')
34 + if os.path.exists(path):
35 + logging.config.fileConfig(path)
36 + break
You should be able to just use get_config() for this (see lava_dispatcher/config.py)

477 +[formatters]
478 +keys: detailed,simple
479 +
480 +[handlers]
481 +keys: console
482 +
483 +[loggers]
484 +keys: root
485 +
486 +[formatter_simple]
487 +format:<LAVA_DISPATCHER>%(asctime)s %(levelname)s: %(message)s
488 +datefmt:%Y-%m-%d %I:%M:%S %p
489 +
490 +[formatter_detailed]
491 +format:<LAVA_DISPATCHER>%(asctime)s %(levelname)s %(module)s:%(lineno)d: %(message)s
492 +datefmt:%Y-%m-%d %I:%M:%S %p
493 +
494 +[handler_console]
495 +class: StreamHandler
496 +args: []
497 +formatter: simple
498 +
499 +[logger_root]
500 +level: DEBUG
501 +name: LAVA_DISPATCHER
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.

Otherwise, looking much better!

review: Needs Fixing

« Back to merge proposal