chore: remove structlog #635
Conversation
projects/pgai/pgai/logger.py
Outdated
| def get_logger(name: str = "") -> logging.Logger: | ||
| """Get a logger instance with the pgai namespace. | ||
|
|
||
| Args: | ||
| name: The logger name, which will be prefixed with 'pgai.' | ||
|
|
||
| Returns: | ||
| A Logger instance with the appropriate namespace | ||
| """ | ||
| if name: | ||
| logger_name: str = f"pgai.{name}" | ||
| else: | ||
| logger_name: str = "pgai" | ||
|
|
||
| return logging.getLogger(logger_name) |
There was a problem hiding this comment.
I don't understand how this is any different to "just" using logging.getLogger(__name__)?
There was a problem hiding this comment.
it enforces the pgai prefix
There was a problem hiding this comment.
The prefix is always pgai (except in __main__.py), because all files are in the pgai module.
projects/pgai/pgai/logger.py
Outdated
| def set_level(level: int | str) -> None: | ||
| """Set the log level for all pgai loggers. | ||
|
|
||
| This does not affect the root logger or any other loggers outside | ||
| the pgai namespace. | ||
|
|
||
| Args: | ||
| level: The logging level (e.g., logging.INFO, logging.DEBUG) | ||
| or a string level name ('INFO', 'DEBUG', etc.) | ||
| """ | ||
| if isinstance(level, str): | ||
| numeric_level: int = getattr(logging, level.upper(), logging.INFO) | ||
| else: | ||
| numeric_level = level | ||
|
|
||
| logging.getLogger("pgai").setLevel(numeric_level) |
There was a problem hiding this comment.
I'm not sure I understand the utility of this. The caller of this could just as well use logging.getLogger("pgai").setLevel() themselves, which is the standard way of doing this in python logging.
There was a problem hiding this comment.
yeah I can get rid of this
projects/pgai/pgai/logger.py
Outdated
|
|
||
| @staticmethod | ||
| def default_renderer(msg: str, kwargs: dict[str, Any]) -> str: | ||
| return f"{msg} >>> {json.dumps(kwargs)}" |
There was a problem hiding this comment.
This fails when the vectorizer worker runs, because json.dumps can't serialize UUID.
| RendererType = Callable[[str, dict[str, Any]], str] | ||
|
|
||
|
|
||
| class StructuredMessage: |
There was a problem hiding this comment.
I don't understand why we're introducing the StructuredMessage class. I am aware that we had structured logging with structlog, but I don't understand if we're just trying to replicate what structlog had, or provide some functionality for our users.
There was a problem hiding this comment.
This is really mostly for internal usage to be able to keep some of the structured data we already specified in the output
There was a problem hiding this comment.
This change breaks logging for the install subcommand (no logs appear).
45eec89 to
0e61ee2
Compare
It is considered best practice for Python libraries to use the standard logger package.
0e61ee2 to
9111bc0
Compare
9111bc0 to
a910278
Compare
35e80a5 to
f15011e
Compare
It is considered best practice for Python libraries to use the standard logger package: https://docs.python.org/3/howto/logging.html#library-config.