Skip to content

Conversation

eschutho
Copy link
Member

SUMMARY

This PR adds test coverage for the log_task_failure signal handler that was introduced in #35595.

The original PR added the signal handler to log Celery task failures, but did not include tests. This follow-up PR adds comprehensive test coverage for the handler.

Related to #35595

TESTING INSTRUCTIONS

Run the new tests:

pytest tests/integration_tests/reports/scheduler_tests.py::test_log_task_failure_with_sender tests/integration_tests/reports/scheduler_tests.py::test_log_task_failure_without_sender -v

Both tests should pass, verifying:

  • The handler logs correctly when a task sender is provided
  • The handler gracefully handles None sender and logs "Unknown"

ADDITIONAL INFORMATION

  • Has associated issue: Related to fix: Log Celery task failures with a signal handler #35595
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

Added unit tests to verify the log_task_failure signal handler:
- Test logging with sender task provided
- Test logging when sender is None (fallback to "Unknown")

Both tests verify the correct logging behavior with proper exception info.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

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

I've completed my review and didn't find any issues.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

@dosubot dosubot bot added the alert-reports Namespace | Anything related to the Alert & Reports feature label Oct 17, 2025
@eschutho eschutho merged commit 1b6d57c into master Oct 20, 2025
68 of 70 checks passed
@eschutho eschutho deleted the elizabeth/task-retry-tests branch October 20, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.0:checkpoint alert-reports Namespace | Anything related to the Alert & Reports feature size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants