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.

Sign up for GitHub

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

Jump to bottom

changed log level of exceptions happening in tasks from WARNING to ERROR #486

Merged
merged 5 commits into from
Nov 10, 2024
Merged

changed log level of exceptions happening in tasks from WARNING to ERROR #486

merged 5 commits into from
Nov 10, 2024

Conversation

syncblaze
Copy link
Contributor

@syncblaze syncblaze commented Nov 8, 2024 •
edited
Loading

Summary

changed log level of exceptions happening in tasks from WARNING to ERROR

Checklist

  • I have run nox and all the pipelines have passed.
  • I have created a changelog fragment to describe this change

Related issues

@codecov Codecov
Copy link

codecov bot commented Nov 8, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 53.63%. Comparing base (9b6d467) to head (c60585f).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
lightbulb/tasks.py 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master     #486   +/-   ##
=======================================
  Coverage   53.63%   53.63%           
=======================================
  Files          37       37           
  Lines        2905     2905           
=======================================
  Hits         1558     1558           
  Misses       1347     1347

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@syncblaze
Copy link
Contributor Author

i would assume a fragment is not needed so i just ignored that, if it needs one tell me i'll add it

null-domain
null-domain reviewed Nov 8, 2024
View reviewed changes
Copy link
Collaborator

@null-domain null-domain left a comment

Choose a reason for hiding this comment

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

LGTM, and I agree with what you said on Discord -

wouldnt it make much more sense to change the logging level of exceptions happening inside of tasks to error instead of warning? when an exception happens while loading extensions, the logging level is ERROR, when an exception happens during the execution of an command the level is ERROR but inside a task it logs as warning

Definitely seems like an inconsistency to me!

Personally I would say to include the fragment anyway - while it's such a small tweak that changes nothing functionally, it's still good to have it documented in the changelog.

lightbulb/tasks.py Outdated Show resolved Hide resolved
@syncblaze
Copy link
Contributor Author

will add the fragment in a few minutes

@syncblaze
Copy link
Contributor Author

omg i am so stupid bufix 😭

it was 4 am dont judge me please 💀

@null-domain null-domain merged commit bd210d9 into tandemdude:master Nov 10, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@null-domain null-domain null-domain left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@syncblaze @null-domain