-
Notifications
You must be signed in to change notification settings - Fork 222
Check and drop if stale connection when saving task result in db #498
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds protection against stale database connections when storing Celery task results. It calls Django's close_if_unusable_or_obsolete() method before saving results to ensure the connection is still valid, particularly for long-running tasks that may exceed the configured connection max age.
Key Changes:
- Added connection validation logic in
_store_result()to close stale connections before saving - Added test coverage to verify stale connections are properly detected and replaced
- Configured
CONN_MAX_AGE: Nonefor test databases to enable persistent connection testing
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
django_celery_results/backends/database.py |
Added close_if_unusable_or_obsolete() call and imported connections from django.db |
t/unit/backends/test_database.py |
Added test to validate stale connection handling, imported time and connections |
t/proj/settings.py |
Configured CONN_MAX_AGE: None for all test database connections |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@auvipy the copilot description beat me to the punch: I noticed some of our very long running celery tasks getting this exception - the work they do is not on the db connection of the task thread, and so that connection remains idle until the end of the task, at which point the server already kicked it off for being idle for too long. Main consequence is that I had to modify the tests to run outside of the usual pytest-django transaction, since the connection dropping does not play nice with being inside a transaction. |
No description provided.