aniketwaghh opened a new issue, #72694:
URL: https://github.com/apache/airflow/issues/72694
### Apache Airflow version
main (`31117014f3`)
### What happened and how to reproduce it?
`td_format` renders a negative duration as a large positive one. A negative
hour comes back as nearly thirty days.
```python
from datetime import timedelta
from airflow._shared.timezones.timezone import td_format
td_format(timedelta(seconds=-3752)) # '29d:22h:57M:28s'
td_format(timedelta(days=-5)) # '25d'
td_format(timedelta(seconds=-0.4)) # '29d:23h:59M:59s'
td_format(-3752) # '<1s'
```
The last two lines are the same duration expressed two ways, and they
disagree: the `timedelta` branch says thirty days, the numeric branch says
under a second.
The cause is the day-to-month conversion:
```python
months, delta.days = divmod(delta.days, 30)
```
`divmod` floors, so a `relativedelta` of `days=-1` becomes `months=-1,
days=+29`. `_format_part` then drops any component below 1:
```python
value = int(getattr(delta, key))
if value < 1:
return ""
```
which removes the `-1 month` that would have cancelled the `+29 days`, and
leaves the days behind. Stepping through `timedelta(seconds=-3752)`:
```
relativedelta(-3752s) -> days=-1, hours=+22, minutes=+57, seconds=+28
divmod(-1, 30) -> (-1, 29)
after normalize -> months=-1, days=+29, hours=+22, minutes=+57,
seconds=+28
months = -1 -> dropped by `value < 1`
days = +29 -> kept
```
The numeric branch differs because `relativedelta(seconds=-3752)` keeps a
single negative `seconds` field rather than borrowing a day, so every component
is negative, every one is dropped, and the empty result falls through to `<1s`.
### What you think should happen instead?
A negative duration should read as a negative duration — the magnitude
formatted as it is today with a leading `-` — and both input types should agree
for the same duration. `timedelta(seconds=-0.4)` should give `<1s` the way
`timedelta(seconds=0.4)` does.
### Anything else?
Worth being straight about the impact: nothing in the tree calls `td_format`
today. It arrived in #20112 for the duration column in the old Flask views, and
the React UI replaced those. So this is latent rather than user-visible right
now.
Two things still argue for fixing it. It lives in `shared/timezones`, which
is vendored into both distributions — `airflow._shared.timezones` and
`airflow.sdk._shared.timezones` — so it is importable by anything that depends
on either. And #62123 fixed the zero-duration behaviour of this same function
in February, touching exactly the two files this would touch, so the function
is evidently still maintained.
Negative durations are not hypothetical in a distributed scheduler: a task
whose recorded end precedes its start, from clock skew between components,
produces one.
### Are you willing to submit PR?
Yes — happy to put one up (sign preserved, both branches agreed,
parametrised tests alongside the existing `test_td_format`). Say the word if
you would rather leave an uncalled helper alone and I will drop it.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
