gopidesupavan commented on issue #50911: URL: https://github.com/apache/airflow/issues/50911#issuecomment-3104294746
> [@gopidesupavan](https://github.com/gopidesupavan) Sweet, will do. I looked into modifying the `_get_next` function today rather than the one I originally mentioned. However, everything looks good in this function provided that a `DateTime` is passed in, and similarly up the stack trace. > > I think the real issue is in the original spot I mentioned. The `DataInterval` is typed to take a `DateTime` instance for both arguments, but `dag_run.data_interval_start` and `dag_run.data_interval_end` are `datetime` instances. Based on the type hints given in the functions, everything looks right all the way up the stack trace except this spot. > > I could change the `_cron` `_get_next` function, but if I do, I feel the correct thing would be to change the typing to be `datetime | DateTime` and that would percolate a lot of changes to type hints throughout. Since these methods are designed to take a `DateTime` instance, I think the correct thing to do is just modify `latest_only` where we are passing the incorrect type. > > If you feel strongly that `_get_next` and `_get_prev` should be changed to take `DateTime | datetime` with a conversion to `pendulum.instance` for any `datetime` passed in, I can make the changes, but this will take me much longer, as I should look at the functions that call them. > > I've made the PR at [#53541](https://github.com/apache/airflow/pull/53541) with the fix I originally proposed, but like I said, happy to make the change directly to `_cron` if you feel those functions really should take a `datetime` or `DateTime` as the argument. thanks will look into, was off sick last two days. -- 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]
