Code review comment for lp:~elopio/u1-test-utils/logger_decorator

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

I made some tweaks to this logging function and pasted them here:

https://pastebin.canonical.com/89011/

I added comments inline explaining the changes, but will duplicate those here for the sake of the review:

"Creating several strings depending on conditions is expensive.
Also, is better to have non-chaging log messages to be able to
grep for specific keywords when debugging, so, I prefer to always
log the args and kwargs even if they are not set, since the fact
that no args nor kwargs were given is an important info to log.
Is particularly important to use %r and not %s.

You should not format the string to log outside the logging
function, for mainly 2 reasons: the formatting may not be needed
since the currently set log level may be higher than the level of
log_func, and because there may be a formatting error that could
raise an exception making the whole call to 'f' fail (and we want
to avoid that)."

review: Needs Fixing

« Back to merge proposal