Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔊(prod) move logging config up to Base configuration class #719

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

Morendil
Copy link
Contributor

Purpose

Make it possible to set logging configuration on a per-deployment basis in production, so we can actually see our application logs in our production environment.

Proposal

Currently, LOGGING is defined only on the Development class. This means that the Staging, PreProduction and Production classes, which are used in our prod environments, do not define any logger options. As a result, our production environments use the default Python logging configuration: a log level of WARNING, and no timestamping of logs, among other things.

This PR moves up the LOGGING definition, to the Base class, this way Production and subclasses will inherit it. This will make INFO the default log level for the root and app loggers. This definition includes an environment variable mapping, so we can still configure a different log level for those if necessary.

Copy link
Member

@qbey qbey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix :)

This move makes it possible to set logging configuration on a per-deployment
basis in production.
@Morendil Morendil force-pushed the lbo/move-log-config-up branch from 8583d01 to b8b731d Compare February 12, 2025 13:43
@Morendil Morendil merged commit d08198e into main Feb 12, 2025
18 of 19 checks passed
@Morendil Morendil deleted the lbo/move-log-config-up branch February 12, 2025 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants