official-stockfish / fishtest

The Stockfish testing framework
https://tests.stockfishchess.org/tests
281 stars 130 forks source link

Replace print by the Python logging framework (RFC) #2065

Closed vdbergh closed 3 months ago

vdbergh commented 3 months ago

Log messages with level >= ERROR give an event log message.

Also: move scheduler to a separate file scheduler.py.

ppigazzini commented 3 months ago

If I'm not wrong you are using the root logger, the best practice is to log at the module level, see at example https://betterstack.com/community/guides/logging/python/python-logging-best-practices/

class ActionDb:
    def __init__(self, db):
        self.db = db
        self.actions = self.db["actions"]
        self.logger = logging.getLogger(__name__)
vdbergh commented 3 months ago

I am not using the root logger but a child logger with name "fishtest". I am not using __name__ since I am using the same logger everwhere.

I had quite a bit of trouble with this PR since the tutorials did not work. The reason seems to be that pserve already does some configuration. Switching to a child logger fixed things.

EDIT: The reason I am using the same logger everywhere is that loggers seem to require some configuration and I do not want to repeat this.

ppigazzini commented 3 months ago

Pyramid example logs at module level, https://docs.pylonsproject.org/projects/pyramid/en/latest/narr/logging.html

The logger configuration in Pyramid is in the usual ini files:

https://github.com/official-stockfish/fishtest/blob/322eea2e377023df3393d577587b9100666949f0/server/production.ini#L37-L67

https://github.com/official-stockfish/fishtest/blob/322eea2e377023df3393d577587b9100666949f0/server/development.ini#L32-L62

vdbergh commented 3 months ago

Hmm that's nice! It seems pyramid already does some of the heavy lifting.

It is not clear to me however how one declares a project specific handler. I am using a handler that posts to the event log.

vdbergh commented 3 months ago

https://stackoverflow.com/questions/30301824/add-custom-log-handler-to-pyramid

vdbergh commented 3 months ago

@ppigazzini The default logging configuration precedes every line with a time stamp. But your logs already seem to have timestamps (provide by systemd?). So I assume you don't want timestamps in the pyramid logs. Is this correct?

ppigazzini commented 3 months ago

Systemd already add the time stamp.

vdbergh commented 3 months ago

Another problem is that currently the constructor for EventHandler takes rundb.actiondb as argument (which itself is constructed inside rundb.init(). It is not obvious to me how handle this without resorting to global variables.

I guess the EvenHandler will have to open a new connection to the db in the constructor. Not a big deal.

ppigazzini commented 3 months ago

If the best practice is to log at module level, log is a global variable that somewhat replace the print function:

def foo():
    message = "Error!"
    print(message)
import logging
log = logging.getLogger(__name__)

def bar():
    message = "Error!"
    log.debug(message)
vdbergh commented 3 months ago

The main motivation for this PR is that I want to define a custom logger that also logs to the event log for level >= ERROR. To replace the pattern

message="error"
print(message, flush=True)
self.actiondb.log_message(fishtest.system, message)

by

message="error"
logger.error(message)

Unfortunately I don't know how to do define a custom handler in the Pyramid framework that takes argument (I tried the last two hours or so but there are errors I don't understand).

vdbergh commented 3 months ago

I think I am getting it to work now.

vdbergh commented 3 months ago

Closing in favor of #2067

vdbergh commented 3 months ago

This the code for the custom handler...

class EventHandler(logging.Handler):
    def __init__(self):
        super().__init__(level=logging.ERROR)
        logging.disable(level=logging.CRITICAL)
    rundb = RunDb(is_primary_instance=False)
        logging.disable(level=logging.NOTSET)
        self.actiondb = rundb.actiondb

    def emit(self, record):
        message=self.format(record)
    self.actiondb.log_message(
            username="fishtest.system",
            message=message,
        )