Makes _on_retry return a float as required to be used as errback on retry_over_time #9741
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9741 +/- ##
=======================================
Coverage 78.51% 78.51%
=======================================
Files 153 153
Lines 19127 19129 +2
Branches 2533 2533
=======================================
+ Hits 15018 15020 +2
+ Misses 3821 3819 -2
- Partials 288 290 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@Nusnus Another issue related to |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the _on_retry method in DelayedDelivery to return a float value as required by the retry_over_time errback, preventing a TypeError when trying to cast a NoneType to a float.
- Updated _on_retry method in delayed_delivery.py to return an interval as a float
- Added a unit test to confirm that _on_retry now returns a float
- Updated CONTRIBUTORS.txt with a new contributor
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| t/unit/worker/test_native_delayed_delivery.py | Added a unit test to verify _on_retry returns a float |
| celery/worker/consumer/delayed_delivery.py | Modified _on_retry to return a float instead of None |
| CONTRIBUTORS.txt | Added Lucas Infante as a contributor |
auvipy
left a comment
There was a problem hiding this comment.
the change seems to be a good one. can you please check the smoke tests failures?
@auvipy, I increased the test timeout and it looks like it worked. |
…on affected tests
|
@auvipy Hey! I don't totally understand what is going on with the smoke tests timing out, so I decided to completely remove my changes (you can check it here e8eaef4) and give smoke tests a run to double check it. It looks like the timeouts are intermittent and unrelated to my changes, so I'm not sure how to properly deal with them. Any tips? Maybe @Nusnus has some hints? I'll put my changes back but the smoke tests are still likely to fail. |
|
the smoke test time outs are regular occurance. dont worry about it |
The
_on_retrymethod fromDelayedDeliveryis still not compatible with the requirements to be used as theerrbackonretry_over_timesince it returns None . Reference hereIf we check kombu code, it's expected for the errback callable to return a float or an object that can be cast into a float. Reference here.
This causes the setup of delayed delivery to completely fail if it encounters an issue on its first try, because it will not be able to retry due to a
TypeErrorwhen trying to cast aNoneTypeinto a float.This PR aims to fix the return type of the _on_retry method to make it compatible with the expected return type for errback.
✅ PR Checklist
pre-commitortox -e lintto ensure style and lint checks pass.