SameerMesiah97 commented on code in PR #64751:
URL: https://github.com/apache/airflow/pull/64751#discussion_r3808097308
##########
airflow-core/src/airflow/models/deadline_alert.py:
##########
@@ -50,13 +50,22 @@ class DeadlineAlert(Base):
name: Mapped[str | None] = mapped_column(String(250), nullable=True)
description: Mapped[str | None] = mapped_column(Text, nullable=True)
reference: Mapped[dict] = mapped_column(JSON, nullable=False)
- interval: Mapped[float] = mapped_column(Float, nullable=False)
+ interval: Mapped[dict] = mapped_column(JSON, nullable=False)
callback_def: Mapped[dict] = mapped_column(JSON, nullable=False)
def __repr__(self):
- interval_seconds = int(self.interval)
- if interval_seconds >= 3600:
+ interval_seconds = None
+
+ if isinstance(self.interval, (int, float)):
+ interval_seconds = int(self.interval)
+
+ elif isinstance(self.interval, datetime.timedelta):
+ interval_seconds = int(self.interval.total_seconds())
+
+ if interval_seconds is None:
+ interval_display = "dynamic"
+ elif interval_seconds >= 3600:
Review Comment:
@feruzzi
If you check the current `main` branch, you can see this has already been
addressed:
```
def __repr__(self):
interval_seconds = None
# Legacy rows store a bare number instead of a serialized dict.
if isinstance(self.interval, (int, float)):
interval_seconds = int(self.interval)
elif isinstance(self.interval, dict):
data = self.interval.get("__data__")
if isinstance(data, (int, float)):
interval_seconds = int(data)
if interval_seconds is None:
interval_display = "dynamic"
elif interval_seconds >= 3600:
interval_display = f"{interval_seconds // 3600}h"
elif interval_seconds >= 60:
interval_display = f"{interval_seconds // 60}m"
else:
interval_display = f"{interval_seconds}s"
return (
f"[DeadlineAlert] "
f"id={str(self.id)[:8]}, "
f"created_at={self.created_at}, "
f"name={self.name or 'Unnamed'}, "
f"reference={self.reference}, "
f"interval={interval_display}, "
f"callback={self.callback_def}"
)
```
The serialized timedelta representation is now handled directly by reading
its numeric `__data__` value, while dynamic or unexpected serialized values
fall back to dynamic. This also avoids needing to deserialize the interval or
import the SDK `VariableInterval` type in `__repr__`.
The associated test has also been updated with a test case to handle
scenarios where interval is in the form of a `dict`. Pleasse see the below:
```
@pytest.mark.parametrize(
("interval", "expected"),
[
# Post-0117 shape: interval is the serialized dict, not a bare
number.
pytest.param(
{"__classname__": "datetime.timedelta", "__data__": 7200.0},
"interval=2h", id="timedelta_2h"
),
# A corrupted dict without ``__data__`` must still render (no
raise) as dynamic.
pytest.param({"unexpected": "shape"}, "interval=dynamic",
id="corrupted_dict_dynamic"),
# A VariableInterval serializes with a dict ``__data__`` (its
key), not a number,
# so it renders as dynamic rather than a fixed duration.
pytest.param(
{
"__classname__":
"airflow.sdk.definitions.deadline.VariableInterval",
"__data__": {"key": "deadline_seconds"},
},
"interval=dynamic",
id="variable_interval_dynamic",
),
],
)
def test_deadline_alert_repr_does_not_raise_on_json_dict_interval(
self, deadline_alert_orm, interval, expected
):
"""``DeadlineAlert.__repr__`` must not raise for the production
JSON-dict interval shape."""
deadline_alert_orm.interval = interval
repr_str = repr(deadline_alert_orm) # must not raise
assert "[DeadlineAlert]" in repr_str
assert expected in repr_str
```
--
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]
