Replaces datetime.fromisoformat with the more lenient dateutil parser#8507
Replaces datetime.fromisoformat with the more lenient dateutil parser#8507auvipy merged 2 commits intocelery:mainfrom stumpylog:main
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #8507 +/- ##
==========================================
+ Coverage 87.44% 87.45% +0.01%
==========================================
Files 148 148
Lines 18491 18499 +8
Branches 3156 3158 +2
==========================================
+ Hits 16169 16179 +10
+ Misses 2033 2032 -1
+ Partials 289 288 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
This change seems to make sense but we need proper testing added. |
Are you still blocked? |
|
Yes, I'm not familiar enough to know where I would add some datetime strings to verify more iso formats can be parsed. Everything I found was using an actual datetime already, so it's format was fine before and after. |
TBH I am also not familiar that much with this part of the code, but check this out, maybe this module is what you're looking for: celery/t/unit/utils/test_time.py Line 99 in ba994d8 Linking to a specific (relevant) function that possibly could help you ease into testing the new PR, but check the rest of the module for additional context @stumpylog |
|
I've added additional coverage of |
Great! Thank you! |
Note: Before submitting this pull request, please review our contributing
guidelines.
Description
Fixes #8499. The
isoparsefrom dateutil is more correct about what it supports, handling the 'Z' timezone as well, which earlier versions of Python didn't support.I'm not 100% sure where the user is providing the eta as a string to, since apply_async notes it to be a datetime. If someone can point me to that and a simple test case, I'll add some additional date strings there to catch this in the future.