Add support for delay and backoff settings#129
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for configurable retry delays with exponential backoff to help handle intermittent database availability issues. When database connection attempts fail, the system can now wait progressively longer between retries.
Changes:
- Added
DBCONN_RETRY_DELAYandDBCONN_RETRY_BACKOFFsettings with defaults of 0 and 1 respectively - Implemented exponential backoff calculation that applies delay before each retry attempt
- Added comprehensive test coverage for the backoff behavior with mocked sleep calls
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| django_dbconn_retry/apps.py | Implements delay and exponential backoff logic in the retry mechanism, reading new settings and applying calculated delays between retry attempts |
| django_dbconn_retry/tests/init.py | Adds test class with test case validating that delays are applied correctly with exponential backoff multiplier |
| README.rst | Documents the two new settings with descriptions, default values, and usage examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @override_settings(MAX_DBCONN_RETRY_TIMES=3, DBCONN_RETRY_DELAY=1.0, DBCONN_RETRY_BACKOFF=2.0) | ||
| @patch('django_dbconn_retry.apps.time.sleep') | ||
| def test_delay_with_backoff(self, mock_sleep: Mock) -> None: | ||
| """Test that delay increases with backoff multiplier.""" | ||
| self.assertRaises(OperationalError, connection.ensure_connection) | ||
| # With 3 retries, delay=1.0, backoff=2.0: | ||
| # retry 1: 1.0 * 2.0^0 = 1.0 | ||
| # retry 2: 1.0 * 2.0^1 = 2.0 | ||
| # retry 3: 1.0 * 2.0^2 = 4.0 | ||
| self.assertEqual(mock_sleep.call_count, 3) | ||
| expected_delays = [1.0, 2.0, 4.0] | ||
| for i, call in enumerate(mock_sleep.call_args_list): | ||
| self.assertEqual(call[0][0], expected_delays[i]) | ||
| del connection._connection_retries |
There was a problem hiding this comment.
The test coverage only validates the exponential backoff case with both non-default values. Consider adding test cases for edge cases such as: 1) DBCONN_RETRY_DELAY=0 (should not call sleep at all), and 2) DBCONN_RETRY_BACKOFF=1 (should apply constant delay without exponential growth).
There was a problem hiding this comment.
I added a couple tests to test the default behaviour of no delay and constant backoff. This should work to test the edge cases mentioned here as well as the defaulting behaviour
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@jdelic sorry to prod, but have you had a chance to take a look at this? |
2 participants
This PR adds two new settings,
DBCONN_RETRY_DELAYandDBCONN_RETRY_BACKOFF. These settings allow you to configure dbconn-retry to sleep for some time in between retries -- the DELAY setting specifies a number of seconds to wait and the BACKOFF setting specifies how much the DELAY should multiplicatively increase with each retry. This could help with situations where you have intermittent database availability. Let me know if you think this could be a useful behaviour. Thanks, Shawn