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

Respect worker log level config setting #903

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

soapy1
Copy link
Contributor

@soapy1 soapy1 commented Oct 19, 2024

Description

This small PR updates the conda store worker to respect the CondaStoreWorker.log_level config parameter. Now users can see that the log level set for their works gets applied to the worker.

Pull request checklist

  • Did you test this change locally?
  • Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

How to test

Before this change, the worker was always running at logger.INFO log level. For example, you can see in the log output

. . .
conda-store-worker-1  | [2024-10-19 01:41:55,960: INFO/MainProcess] Connected to redis://:**@redis:6379/0
conda-store-worker-1  | [2024-10-19 01:41:55,962: WARNING/MainProcess] /opt/conda/envs/conda-store-server/lib/python3.12/site-packages/celery/worker/consumer/consumer.py:508: CPendingDeprecationWarning: The broker_connection_retry configuration setting will no longer determine

To test this add the following line to your conda-store config file

c.CondaStoreWorker.log_level = logging.ERROR

Restart the server and view the logs. Notice that the WARN and INFO log lines no longer appear in the logs.

Copy link

netlify bot commented Oct 19, 2024

Deploy Preview for conda-store canceled.

Name Link
🔨 Latest commit a752f6d
🔍 Latest deploy log https://app.netlify.com/sites/conda-store/deploys/671310639769830008fe9f84

Copy link

netlify bot commented Oct 19, 2024

Deploy Preview for conda-store canceled.

Name Link
🔨 Latest commit 04fe23a
🔍 Latest deploy log https://app.netlify.com/sites/conda-store/deploys/67166ab97f126100083bd23e

# celery supports the log levels DEBUG | INFO | WARNING | ERROR | CRITICAL | FATAL
# https://docs.celeryq.dev/en/main/reference/cli.html#celery-worker
logging_to_celery_level_map = {
50:"CRITICAL", 50:"FATAL", 40: "ERROR", 30: "WARNING", 20: "INFO", 10: "DEBUG"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't really a 1:1 relationship between the logging package error levels and the expected logging levels for celery. So, I think maintaining a map is required. Definitely open to suggestions if there is a better place to put this, like maybe a utils type file that I missed.

@soapy1 soapy1 marked this pull request as draft October 21, 2024 14:50
@soapy1 soapy1 marked this pull request as ready for review October 21, 2024 16:32
@peytondmurray peytondmurray self-requested a review October 24, 2024 17:10
Copy link
Contributor

@peytondmurray peytondmurray left a comment

Choose a reason for hiding this comment

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

This definitely looks correct, but given that Nebari may be impacted I'm going to wait for the okay from Nebari devs before merging this.

@peytondmurray
Copy link
Contributor

Confirmed after talking with @viniciusdc that this should be safe to do. Thanks!

@peytondmurray peytondmurray merged commit 6fb6d6a into conda-incubator:main Oct 24, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

2 participants