ashb commented on a change in pull request #14212:
URL: https://github.com/apache/airflow/pull/14212#discussion_r577478212
##########
File path: airflow/models/taskinstance.py
##########
@@ -927,7 +927,7 @@ def next_retry_datetime(self):
delay_backoff_in_seconds = min(modded_hash,
timedelta.max.total_seconds() - 1)
delay = timedelta(seconds=delay_backoff_in_seconds)
if self.task.max_retry_delay:
- delay = min(self.task.max_retry_delay, delay)
+ delay = timedelta(seconds=min(self.task.max_retry_delay,
delay.total_seconds()))
Review comment:
This change isn't needed -- task.max_retry_delay should be a timedelta)
We should check this in the ctor of BaseOperator, and if what is passed is
not a timedleta then do:
```python
if not isinstance(max_retry_delay):
max_retry_delay = timedelta(seconds=max_retry_delay)
```
etc.
Could you add a test to tests/models/test_baseoperator.py that checks this
upgrade behaviour?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
