dhruv-techdev commented on code in PR #75530:
URL: https://github.com/apache/airflow/pull/75530#discussion_r4464007550
##########
providers/standard/src/airflow/providers/standard/operators/datetime.py:
##########
@@ -110,5 +111,9 @@ def target_times_as_dates(
return lower, upper
if upper < lower:
- upper += datetime.timedelta(days=1)
+ # Past midnight, the window that contains base_date started the
previous day
+ if lower_is_time and base_date.time() <= upper.time():
+ lower -= datetime.timedelta(days=1)
+ else:
+ upper += datetime.timedelta(days=1)
Review Comment:
Yes, good spot, `target_upper` needs a guard too. That line was already
there before this PR, but it's the same kind of bug, so I'll fix it here.
For example, take `target_lower=time(22, 0)` and
`target_upper=datetime(2020, 7, 7, 2, 0)`. A run at 2020-07-07 23:00 takes the
true branch because `target_upper` gets moved to Jul 8 02:00, but it should
take the false branch, since 23:00 is past the end the user set.
I also need to correct my last comment, because not all mixed targets behave
as they did before. With the same targets, a run at 2020-07-07 01:00 used to
take the false branch and now takes the true branch, because `target_lower`
moves back to Jul 6 22:00. I think that's the better result, since 01:00 falls
between 22:00 the night before and the exact end, so I'd like to keep it. Let
me know if you'd prefer the old behavior.
I'll change it so `target_upper` only moves forward when it was passed as a
`time`, which means an exact datetime never moves. I'll push that shortly with
tests for both cases.
--
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]
