ferruzzi commented on code in PR #73294:
URL: https://github.com/apache/airflow/pull/73294#discussion_r4067254778


##########
task-sdk/src/airflow/sdk/definitions/deadline.py:
##########
@@ -154,14 +154,22 @@ def __init__(
         callback: Callback,
         name: str | None = None,
     ):
+        if not isinstance(interval, (timedelta, VariableInterval)):

Review Comment:
   Yeah, that's fair.   Supporting `int` was an accidental side effect, but it 
_does_ currently work, so we should clear that up.  There's no reason to use an 
`int` or to expect it to work.  The documentation all lists it as a 
`timedelta`.  I can maybe see where a float might come from since the pre-3.3.0 
column was `sa.Float()` holding total_seconds().  I'm not sure where or why the 
`int` idea comes from originally, but it's in a few places in the code now, so 
it should be addressed either way.
   
   I'm proposing a third option: deprecate it and keep the backport label.  
Accept `int` and `float` as seconds, still reject bool to match your decoder, 
and emit `RemovedInAirflow4Warning` in case anyone is relying on it.



-- 
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]

Reply via email to