You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/06/29 15:28:22 UTC

[GitHub] [airflow] Gollum999 opened a new issue, #24736: TimeSensorAsync breaks if target_time is timezone-aware

Gollum999 opened a new issue, #24736:
URL: https://github.com/apache/airflow/issues/24736

   ### Apache Airflow version
   
   2.3.2 (latest released)
   
   ### What happened
   
   `TimeSensorAsync` fails with the following error if `target_time` is aware:
   ```
   [2022-06-29, 05:09:11 CDT] {taskinstance.py:1889} ERROR - Task failed with exception
   Traceback (most recent call last):
     File "/opt/conda/envs/production/lib/python3.9/site-packages/airflow/sensors/time_sensor.py", line 60, in execute
       trigger=DateTimeTrigger(moment=self.target_datetime),
     File "/opt/conda/envs/production/lib/python3.9/site-packages/airflow/triggers/temporal.py", line 42, in __init__
       raise ValueError(f"The passed datetime must be using Pendulum's UTC, not {moment.tzinfo!r}")
   ValueError: The passed datetime must be using Pendulum's UTC, not Timezone('America/Chicago')
   ```
   
   ### What you think should happen instead
   
   Given the fact that `TimeSensor` correctly handles timezones (#9882), this seems like a bug.  `TimeSensorAsync` should be a drop-in replacement for `TimeSensor`, and therefore should have the same timezone behavior.
   
   ### How to reproduce
   
   ```
   #!/usr/bin/env python3
   import datetime
   
   from airflow.decorators import dag
   from airflow.sensors.time_sensor import TimeSensor, TimeSensorAsync
   import pendulum
   
   
   @dag(
       start_date=datetime.datetime(2022, 6, 29),
       schedule_interval='@daily',
   )
   def time_sensor_dag():
       naive_time1 = datetime.time( 0,  1)
       aware_time1 = datetime.time( 0,  1).replace(tzinfo=pendulum.local_timezone())
       naive_time2 = pendulum.time(23, 59)
       aware_time2 = pendulum.time(23, 59).replace(tzinfo=pendulum.local_timezone())
   
       TimeSensor(task_id='naive_time1', target_time=naive_time1, mode='reschedule')
       TimeSensor(task_id='naive_time2', target_time=naive_time2, mode='reschedule')
       TimeSensor(task_id='aware_time1', target_time=aware_time1, mode='reschedule')
       TimeSensor(task_id='aware_time2', target_time=aware_time2, mode='reschedule')
   
       TimeSensorAsync(task_id='async_naive_time1', target_time=naive_time1)
       TimeSensorAsync(task_id='async_naive_time2', target_time=naive_time2)
       TimeSensorAsync(task_id='async_aware_time1', target_time=aware_time1)  # fails
       TimeSensorAsync(task_id='async_aware_time2', target_time=aware_time2)  # fails
   
   dag = time_sensor_dag()
   ```
   
   This can also happen if the `target_time` is naive and `core.default_timezone = system`.
   
   ### Operating System
   
   CentOS Stream 8
   
   ### Versions of Apache Airflow Providers
   
   N/A
   
   ### Deployment
   
   Other
   
   ### Deployment details
   
   Standalone
   
   ### Anything else
   
   _No response_
   
   ### Are you willing to submit PR?
   
   - [ ] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
   


-- 
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: commits-unsubscribe@airflow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] uranusjr commented on issue #24736: TimeSensorAsync breaks if target_time is timezone-aware

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #24736:
URL: https://github.com/apache/airflow/issues/24736#issuecomment-1170781178

   Perhaps we could allow all Pendulum timezones instead of requiring UTC. Not sure.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] andrewgodwin commented on issue #24736: TimeSensorAsync breaks if target_time is timezone-aware

Posted by GitBox <gi...@apache.org>.
andrewgodwin commented on issue #24736:
URL: https://github.com/apache/airflow/issues/24736#issuecomment-1171370209

   I don't see a problem with adding timezone conversion code to convert it to UTC at call time - I just add UTC-only checks to all timezone items by default if I didn't specifically write timezone support, so there's a nice clean error rather than weird 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] uranusjr commented on issue #24736: TimeSensorAsync breaks if target_time is timezone-aware

Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #24736:
URL: https://github.com/apache/airflow/issues/24736#issuecomment-1170781880

   Convert to a feature since not allowing non-UTC is an explicit design choice made in the current implementation. Need some context on this to decide what best to do here @andrewgodwin 


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk closed issue #24736: TimeSensorAsync breaks if target_time is timezone-aware

Posted by GitBox <gi...@apache.org>.
potiuk closed issue #24736: TimeSensorAsync breaks if target_time is timezone-aware
URL: https://github.com/apache/airflow/issues/24736


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org