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

Increase cssutils logging level to ERROR #85

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

amureki
Copy link
Member

@amureki amureki commented Aug 1, 2024

cssutils is doing some shady things to the logger singleton, that is why it cannot be properly controlled via Django logger (yet).

Thus, I am increasing the current level to ERROR until the issue is resolved.

@amureki amureki added the bug Something isn't working label Aug 1, 2024
@amureki amureki requested a review from codingjoe August 1, 2024 14:50
@amureki amureki self-assigned this Aug 1, 2024
Copy link
Member

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

I am not sure if I understood how one is related to the other and what problem we are experiencing right now. Would you care to elaborate a little?

@amureki
Copy link
Member Author

amureki commented Aug 2, 2024

I am not sure if I understood how one is related to the other and what problem we are experiencing right now. Would you care to elaborate a little?

This is about an extensive logging coming from premailer.
In our platform logging setup, we want to only see ERROR level logs from cssutils.

Instead we are receiving on every email render a bunch of logs like:

 logger=CSSUTILS host=voiio.app: Property: Unknown Property name. [12:13: -webkit-text-size-adjust]"

I do not find these useful and want to drop them.

Copy link
Member

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

If you want to decrease the level because it creates white noise with the templates we provide here, this seems OK. If it's for the same of the platform, I'd prefer to resolve the issue upstream so we projects can override the log level.

@amureki
Copy link
Member Author

amureki commented Aug 5, 2024

If you want to decrease the level because it creates white noise with the templates we provide here, this seems OK. If it's for the same of the platform, I'd prefer to resolve the issue upstream so we projects can override the log level.

I tried to fix the issue in the cssutils library itself, but the logging setup there is convoluted, and I'd need more time to untie it.
Our templates in emark are currently raising the warnings, so I want to fix it here.

Providing a configurable would work, but again, I'd rather want cssutils play the rules and not fixing every (sub-)package that uses it.

@amureki amureki merged commit c54ad29 into main Aug 5, 2024
13 checks passed
@amureki amureki deleted the disable-extensive-cssutil-logging branch August 5, 2024 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants