Skip to content

Fix logger error message with partial callbacks #19609

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix logger error message with partial callbacks #19609

wants to merge 1 commit into from
+69 −8

Conversation

krishna-holvi
Copy link

@krishna-holvi krishna-holvi commented Jun 30, 2025 •
edited
Loading

Trac ticket number

ticket-36487

Branch description

This is a trivial change to fix the logger error message. Fixed the error message for the case on commit callback is a partial which doesn't have the property __qualname__.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

@github-actions github-actions bot added the no ticket Based on PR title, no linked Trac ticket label Jun 30, 2025
github-actions[bot]
github-actions bot reviewed Jun 30, 2025
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

@charettes
Copy link
Member

charettes commented Jun 30, 2025 •
edited
Loading

Hello @krishna-holvi, the problem seems legitimate but just like any non-trivial bug fixing this will require a ticket as well as tests.

See 4a1150b to get an idea for where the tests should be added. It might also be an opportunity to change the logging.error(exc_info=True) calls to logger.exception instead as the latter is an alias for the former.

@krishna-holvi
Copy link
Author

Hello @krishna-holvi, the problem seems legitimate but just like any non-trivial bug fixing this will require a ticket as well as tests.

See 4a1150b to get an idea for where the tests should be added. It might also be an opportunity to change the logging.error(exc_info=True) calls to logger.exception instead as the latter is an alias for the former.

Many thanks for having a look and thank you for those links. I'll update the PR

@krishna-holvi krishna-holvi force-pushed the krishna-fix-logger-partial-error branch from 3426f37 to a8db985 Compare June 30, 2025 15:17
@charettes
Copy link
Member

Looks like it caused a few failures

See

Try running this particular suite locally

@krishna-holvi krishna-holvi force-pushed the krishna-fix-logger-partial-error branch 6 times, most recently from ae88660 to 7156c8a Compare July 1, 2025 07:43
@krishna-holvi krishna-holvi force-pushed the krishna-fix-logger-partial-error branch from 7156c8a to b087dca Compare July 1, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

Assignees
No one assigned
Labels
no ticket Based on PR title, no linked Trac ticket
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@krishna-holvi @charettes