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

Make logging level of the django app configurable #2078

Merged
merged 10 commits into from
Jan 2, 2025
Merged

Conversation

yarikoptic
Copy link
Member

ATM we do not have a single DEBUG level log statement, but I hope that logging would be enriched. Adding such control would also allow to make log less talkative if so desired by setting it to WARNING level. But also would simplify addition and use of new log statements. E.g. I would right away add it into dandi-cli docker-compose setup so to help in the future troubleshooting issues like

ATM we do not have a single DEBUG level log statement, but I hope
that logging would be enriched. Adding such control would also allow
to make log less talkative if so desired by setting it to WARNING level.
But also would simplify addition and use of new log statements
@yarikoptic yarikoptic added DX Affects developer experience internal Changes only affect the internal API labels Nov 20, 2024
@yarikoptic yarikoptic requested a review from waxlamp December 3, 2024 16:57
These are not needed here, as they are already part of the setting
definition (that is, `settings.DANDI_LOG_LEVEL` always exists and has
the correct default value if it is not set explicitly).
The [`logging` module
docs](https://docs.python.org/3/library/logging.html#logging.Logger.setLevel)
explain that `setLevel()` can handle the string representations of the
logging levels. This allows the setting value to be directly tied to
what is passed in, and `setLevel()` also does a "validation" on the
value and will refuse to process levels that don't exist.
Copy link
Member

@waxlamp waxlamp left a comment

Choose a reason for hiding this comment

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

I added a few commits to simplify the code. If you're ok with those, let me know and I will merge.

dandiapi/api/apps.py Outdated Show resolved Hide resolved
This needs to be `logging.INFO` so that Sentry can propery construct
breadcrumbs within its error reports.
Copy link
Member

@waxlamp waxlamp left a comment

Choose a reason for hiding this comment

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

A couple more changes to eliminate some bugs.

dandiapi/api/apps.py Outdated Show resolved Hide resolved
dandiapi/api/apps.py Outdated Show resolved Hide resolved
dandiapi/api/apps.py Outdated Show resolved Hide resolved
@yarikoptic yarikoptic requested a review from waxlamp December 17, 2024 15:44
@waxlamp waxlamp merged commit 480c6ec into master Jan 2, 2025
11 checks passed
@waxlamp waxlamp deleted the enh-logging-level branch January 2, 2025 21:01
@dandibot
Copy link
Member

dandibot commented Jan 3, 2025

🚀 PR was released in v0.4.5 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Affects developer experience internal Changes only affect the internal API released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants