[GitHub] [airflow] timkpaine commented on pull request #30083: Account for change in UTC offset when performing next schedule calculations

[via GitHub]

timkpaine commented on PR #30083:
URL: https://github.com/apache/airflow/pull/30083#issuecomment-1481488500

   I agree completely that the bugged behavior is asserted by tests, and that 
we should be careful with a fix. I think a minor version with a new flag to 
restore the current behavior would be best, followed by a minor version with a 
new flag to get the fixed behavior would be next best, followed by waiting to a 
3.0, but that's just my personal opinion. 
   
   But, I do think the behavior is bugged. The code for "fixing DST" doesn't do 
anything, its unclear why it would be added if not to account for this problem 
(@uranusjr you wrote this code so I would've thought by "fix for DST" you meant 
"if I want to run at 9am and 10am, run at 9am and 10am even after a DST change, 
but I could be wrong, in which case im not sure what "fix DST" means). 
   
   <img width="575" alt="Screenshot 2023-03-23 at 12 12 33" 
src="https://user-images.githubusercontent.com/3105306/227266244-30e0fcd6-cc0e-4f6f-a388-91bb32e39363.png";>
   
   > The thing here is the semantic of `schedule="<cron expr>"` (or 
`schedule_interval`, they are the same)
   
   This is incorrect. `schedule="<cron expr>"` is sometimes schedule interval, 
and sometimes schedule fixed. This is the crux of the usage problem, certain 
cron schedules (given non-fixed hours like `1,2` or `1-3`) behave as interval, 
others with single hours behave as fixed. 
   
   > And you can’t dismiss the current behaviour as a bug
   
   I think it's almost certainly a bug. From the airflow docs themselves:
   
   <img width="1051" alt="Screenshot 2023-03-23 at 12 01 36" 
src="https://user-images.githubusercontent.com/3105306/227263117-3ef5c279-3258-4de5-b161-d29c8f2564ce.png";>
   
   From astronomer.io docs:
   <img width="731" alt="Screenshot 2023-03-23 at 12 02 22" 
src="https://user-images.githubusercontent.com/3105306/227263343-2514f198-f284-48bf-9ae0-0e4cbf7be2d3.png";>
   
   Some other samplings from the internet:
   [Incorrect post](https://dobken.nl/posts/post-11/):
   <img width="881" alt="Screenshot 2023-03-23 at 12 05 47" 
src="https://user-images.githubusercontent.com/3105306/227264389-3afc4713-e926-40fb-b7ab-ebea7274d421.png";>
   
   StackOverflow showing how to account for the bug by running extra tasks on 
either side of the intended tasks, clearly not the expected or desired behavior:
   
https://stackoverflow.com/questions/43662571/how-to-properly-handle-daylight-savings-time-in-apache-airflow
   
   
   > the fact it is ensured by test means the behaviour is delibrately designed 
by someone, and likely relied on someone as a feature.
   
   Completely agree, it is always annoying when bugs become relied on as 
features but given that common understanding would expect airflow to account 
for DST, a minority of users relying on a bug should be deprioritized over a 
majority of users expecting the fixed version, with obvious requirements of 
reporting and versioning so that all users have a path forward. 


-- 
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-unsubscr...@airflow.apache.org

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