-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Closed
Labels
Description
Description
Right now, if you try doing something like the below, the timezone is completely ignored:
from pendulum import Time
from airflow.sensors.time_sensor import TimeSensorAsync
import pytz
wait_for_start_of_business = TimeSensorAsync(
task_id="wait_for_business_hours",
target_time=Time(9, 0, tzinfo=pytz.timezone("America/New_York")),
)The timezone is completely ignored.
It would be nice if the timezone were not ignored in this case. I think the change that would need to occur in the code would be to replace this:
aware_time = timezone.coerce_datetime(
datetime.datetime.combine(datetime.datetime.today(), self.target_time, self.dag.timezone)
)with this:
tz = self.target_time.tzinfo or self.dag.timezone
aware_time = timezone.coerce_datetime(
datetime.datetime.combine(datetime.datetime.now(tz=tz), self.target_time, tz)
)This is backwards compatible so long as:
- The
timedoes not have atzinfoor alternatively thetzinfois equal toself.dag.timezone - The
self.dag.timezoneagrees with the system time. This is a weird one, as I would argue the previous behavior was a minor bug: e.g.datetime.datetime.today()running on Jan 2nd at 03:00:00+0600 (i.e. 6 hours ahead of UTC) will output Jan 2nd even though it is Jan 1st in UTC, so if the system is+0600and the DAG is+0000, the sensor will potentially not fire (for example,target_time=time(20, 0)would not fire on Jan 1st, 20:00:00 UTC in this setup).
(While we are at it, TimeSensor is untyped, so some type annotations could be added during this PR.)
Use case/motivation
I have a Slack notification system that needs to wait until 9am EST before it's allowed to fire off notifications. What constitutes 9am EST in UTC time (which is our Airflow server's timezone) differs depending on daylight savings.
Related issues
No response
Are you willing to submit a PR?
- Yes I am willing to submit a PR!
Code of Conduct
- I agree to follow this project's Code of Conduct
joankaradimov