ferruzzi commented on code in PR #70370:
URL: https://github.com/apache/airflow/pull/70370#discussion_r3857514409
##########
airflow-core/src/airflow/serialization/decoders.py:
##########
@@ -224,6 +225,35 @@ def decode_deadline_alert(encoded_data: dict):
)
+def decode_deadline_alert_model(deadline_alert: DeadlineAlertModel) ->
SerializedDeadlineAlert:
+ """
+ Decode a ``DeadlineAlert`` ORM row into its serialized representation.
+
+ :meta private:
+ """
+ return decode_deadline_alert(
+ {
+ DeadlineAlertFields.REFERENCE: deadline_alert.reference,
+ DeadlineAlertFields.INTERVAL: deadline_alert.interval,
+ DeadlineAlertFields.CALLBACK: deadline_alert.callback_def,
+ }
+ )
+
+
+def resolve_deadline_alert_interval(alert: SerializedDeadlineAlert) ->
datetime.timedelta:
+ """
+ Resolve a decoded alert's interval to a ``timedelta``.
+
+ A ``VariableInterval`` reads its Airflow Variable here, so this is only
called at the point
+ a deadline is actually calculated.
+
+ :meta private:
+ """
+ if isinstance(alert.interval, VariableInterval):
+ return alert.interval.resolve()
Review Comment:
resolve() can raise `ValueError` (for example VariableInterval.resolve()
raises on a non-integer), so on the clear path this can abort the dagrun clear
even though there is a valid deadline in the db. Since dagrun create and
dagrun clear both filter through here now, I think we should be catching and
logging here IF we're clearing.
##########
airflow-core/src/airflow/serialization/decoders.py:
##########
@@ -24,6 +24,7 @@
import dateutil.relativedelta
from airflow._shared.module_loading import import_string
+from airflow.sdk.definitions.deadline import VariableInterval
Review Comment:
I'm not sure about this one. We moved this sdk import to the module level
but left the other sdk import below it as a local?
--
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]
