Drop loguru and use builtin logging
#1133
Conversation
|
Results after this PR: When enabling |
lopagela
force-pushed
the
better-logging
branch
2 times, most recently
from
dc59bb2
to
f861183
Compare
| @@ -5154,20 +5136,6 @@ files = [ | |||
| [package.extras] | |||
| test = ["pytest (>=6.0.0)", "setuptools (>=65)"] | |||
|
|
|||
| [[package]] | |||
There was a problem hiding this comment.
Highlighting this one -- It has been removed as a side effect of poetry remove loguru
How should poetry.lock be managed? Should it be generated by some CICD tool? Or maintained by core dev of this project?
There was a problem hiding this comment.
I think we should ignore for the most part, it's just for consistent builds
| @@ -26,6 +27,8 @@ | |||
| CompletionResponseGen, | |||
| ) | |||
|
|
|||
| log = logging.getLogger(__name__) | |||
There was a problem hiding this comment.
tiny NIT but could you rename this to logger (and in the other file too)?
Will be more consistent naming wise for the future.
There was a problem hiding this comment.
Sure sorry - I'm used to Java's Lombok @Log4j2 that injects log variable into classes 😅
I personally preferring log as it's shorter than logger (it makes lines shorter and seems more readable to me), but no problem, changing to logger
|
Great changes, thanks for addressing the old prints we left around. |
Changed the 2 existing `print` in the `private_gpt` code base into actual python logging, stop using loguru (dependency will be dropped in a later commit). Try to use the `key=value` logging convention in logs (to indicate what dynamic values represents, and what is dynamic vs not). Using `%s` log style, so that the string formatting is pushed inside the logger, giving the ability to the logger to determine if the string need to be formatted or not (i.e. strings from debug logs might not be formatted if the log level is not debug) The (basic) builtin log configuration have been placed in `private_gpt/__init__.py` in order to initialize the logging system even before we start to launch any python code in `private_gpt` package (ensuring we get any initialization log formatted as we want to) Disabled `uvicorn` custom logging format, resulting in having uvicorn logs being outputted in our formatted. Some more concise format could be used if we want to, especially: ``` COMPACT_LOG_FORMAT = '%(asctime)s.%(msecs)03d [%(levelname)s] %(name)s - %(message)s' ``` Python documentation and cookbook on logging for reference: * https://docs.python.org/3/library/logging.html * https://docs.python.org/3/howto/logging.html
Result of `poetry remove loguru`
lopagela
force-pushed
the
better-logging
branch
from
ea67988
to
b31b4d3
Compare
printin theprivate_gptcode base into actual python logging, stop using loguru (dependency will be dropped in a later commit).key=valuelogging convention in logs (to indicate what dynamic values represents, and what is dynamic vs not).%slog style, so that the string formatting is pushed inside the logger, giving the ability to the logger to determine if the string need to be formatted or not (i.e. strings from debug logs might not be formatted if the log level is not debug)private_gpt/__init__.pyin order to initialize the logging system even before we start to launch any python code inprivate_gptpackage (ensuring we get any initialization log formatted as we want to)uvicorncustom logging format, resulting in having uvicorn logs being outputted in our formatted.Some more concise format could be used if we want to, especially:
Python documentation and cookbook on logging for reference: