Skip to content

Commit b88cff1

Browse files
author
Sameer Mesiah
committed
Keep VariableInterval.resolve() as a deprecated compatibility shim instead of removing the public method outright. Move SerializedVariableInterval resolution tests into a dedicated deadline serialization test module and add compatibility coverage for the deprecated VariableInterval.resolve() path.
1 parent 4d5fe92 commit b88cff1

8 files changed

Lines changed: 172 additions & 49 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed ``VariableInterval`` deadline intervals to allow zero and negative offsets, matching the existing ``timedelta`` interval semantics. ``VariableInterval`` is now converted to the core-side ``SerializedVariableInterval`` representation during deadline deserialization and resolved during deadline evaluation.

‎airflow-core/newsfragments/71802.improvement.rst‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎airflow-core/src/airflow/serialization/decoders.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,8 @@ def decode_deadline_alert(encoded_data: dict):
215215

216216
if isinstance(deserialized, datetime.timedelta):
217217
interval = deserialized
218+
elif isinstance(deserialized, SerializedVariableInterval):
219+
interval = deserialized
218220
elif isinstance(deserialized, VariableInterval):
219221
interval = SerializedVariableInterval(key=deserialized.key)
220222
else:

‎airflow-core/src/airflow/serialization/definitions/deadline.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
from airflow._shared.timezones import timezone
3030
from airflow.models.deadline import classproperty
31+
from airflow.models.variable import Variable
3132
from airflow.utils.log.logging_mixin import LoggingMixin
3233
from airflow.utils.session import provide_session
3334
from airflow.utils.sqlalchemy import get_dialect_name
@@ -385,7 +386,6 @@ class SerializedVariableInterval:
385386
key: str
386387

387388
def resolve(self) -> timedelta:
388-
from airflow.models.variable import Variable
389389

390390
try:
391391
value = Variable.get(self.key)
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
from __future__ import annotations
19+
20+
from datetime import timedelta
21+
22+
import pytest
23+
24+
from airflow.models.variable import Variable
25+
from airflow.serialization.definitions.deadline import SerializedVariableInterval
26+
27+
28+
class TestVariableInterval:
29+
@pytest.mark.parametrize(
30+
("value", "expected"),
31+
[
32+
("3", timedelta(seconds=3)),
33+
("10", timedelta(seconds=10)),
34+
("05", timedelta(seconds=5)),
35+
("0", timedelta(0)),
36+
("-5", timedelta(seconds=-5)),
37+
],
38+
)
39+
def test_resolve_valid(self, mocker, value, expected):
40+
mocker.patch.object(Variable, "get", return_value=value)
41+
42+
interval = SerializedVariableInterval(key="test_interval")
43+
44+
assert interval.resolve() == expected
45+
46+
@pytest.mark.parametrize(
47+
("value", "raise_missing", "match"),
48+
[
49+
(None, True, "not found"),
50+
("abc", False, "must be an integer"),
51+
("", False, "must be an integer"),
52+
],
53+
)
54+
def test_resolve_invalid(self, mocker, value, raise_missing, match):
55+
if raise_missing:
56+
mocker.patch.object(
57+
Variable,
58+
"get",
59+
side_effect=KeyError("test_interval"),
60+
)
61+
else:
62+
mocker.patch.object(Variable, "get", return_value=value)
63+
64+
interval = SerializedVariableInterval(key="test_interval")
65+
66+
with pytest.raises(ValueError, match=match):
67+
interval.resolve()

‎airflow-core/tests/unit/serialization/test_serialized_objects.py‎

Lines changed: 24 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
)
4444
from airflow.models.connection import Connection
4545
from airflow.models.dag import DAG
46-
from airflow.models.variable import Variable
4746
from airflow.models.xcom_arg import XComArg
4847
from airflow.partition_mappers.identity import IdentityMapper as CoreIdentityMapper
4948
from airflow.partition_mappers.temporal import (
@@ -80,6 +79,7 @@
8079
AsyncCallback,
8180
DeadlineAlert,
8281
DeadlineReference,
82+
VariableInterval,
8383
)
8484
from airflow.sdk.definitions.decorators import task
8585
from airflow.sdk.definitions.operator_resources import Resources
@@ -277,48 +277,6 @@ def equal_serialized_asset(a: SerializedAssetBase | BaseAsset, b: SerializedAsse
277277
return ensure_serialized_asset(a) == ensure_serialized_asset(b)
278278

279279

280-
@pytest.mark.parametrize(
281-
("value", "expected"),
282-
[
283-
("3", timedelta(seconds=3)),
284-
("10", timedelta(seconds=10)),
285-
("05", timedelta(seconds=5)),
286-
("0", timedelta(0)),
287-
("-5", timedelta(seconds=-5)),
288-
],
289-
)
290-
def test_serialized_variable_interval_resolve_valid(mocker, value, expected):
291-
mocker.patch.object(Variable, "get", return_value=value)
292-
293-
interval = SerializedVariableInterval(key="test_interval")
294-
295-
assert interval.resolve() == expected
296-
297-
298-
@pytest.mark.parametrize(
299-
("value", "raise_missing", "match"),
300-
[
301-
(None, True, "not found"),
302-
("abc", False, "must be an integer"),
303-
("", False, "must be an integer"),
304-
],
305-
)
306-
def test_serialized_variable_interval_resolve_invalid(mocker, value, raise_missing, match):
307-
if raise_missing:
308-
mocker.patch.object(
309-
Variable,
310-
"get",
311-
side_effect=KeyError("test_interval"),
312-
)
313-
else:
314-
mocker.patch.object(Variable, "get", return_value=value)
315-
316-
interval = SerializedVariableInterval(key="test_interval")
317-
318-
with pytest.raises(ValueError, match=match):
319-
interval.resolve()
320-
321-
322280
def equal_serialized_deadline_alert(
323281
a: DeadlineAlert | SerializedDeadlineAlert,
324282
b: DeadlineAlert | SerializedDeadlineAlert,
@@ -539,13 +497,33 @@ def test_serialize_deserialize_connection():
539497

540498

541499
@pytest.mark.parametrize("reference", REFERENCE_TYPES)
542-
def test_serialize_deserialize_deadline_alert(reference):
500+
@pytest.mark.parametrize(
501+
("interval", "expected_interval"),
502+
[
503+
pytest.param(
504+
timedelta(hours=1),
505+
timedelta(hours=1),
506+
id="timedelta",
507+
),
508+
pytest.param(
509+
VariableInterval("deadline_seconds"),
510+
SerializedVariableInterval("deadline_seconds"),
511+
id="sdk_variable_interval",
512+
),
513+
pytest.param(
514+
SerializedVariableInterval("deadline_seconds"),
515+
SerializedVariableInterval("deadline_seconds"),
516+
id="serialized_variable_interval",
517+
),
518+
],
519+
)
520+
def test_serialize_deserialize_deadline_alert(reference, interval, expected_interval):
543521
public_deadline_alert_fields = {
544522
field.lower() for field in vars(DeadlineAlertFields) if not field.startswith("_")
545523
}
546524
original = DeadlineAlert(
547525
reference=reference,
548-
interval=timedelta(hours=1),
526+
interval=interval,
549527
callback=AsyncCallback(empty_callback_for_deadline, kwargs=TEST_CALLBACK_KWARGS),
550528
)
551529

@@ -556,7 +534,7 @@ def test_serialize_deserialize_deadline_alert(reference):
556534

557535
deserialized = BaseSerialization.deserialize(serialized)
558536
assert deserialized.reference.serialize_reference() == reference.serialize_reference()
559-
assert deserialized.interval == original.interval
537+
assert deserialized.interval == expected_interval
560538
assert deserialized.callback == original.callback
561539

562540

‎task-sdk/src/airflow/sdk/definitions/deadline.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from __future__ import annotations
1818

1919
import logging
20+
import warnings
2021
from abc import ABC
2122
from dataclasses import dataclass
2223
from datetime import datetime, timedelta
@@ -25,6 +26,8 @@
2526
import attrs
2627

2728
from airflow.sdk.definitions.callback import AsyncCallback, Callback, SyncCallback
29+
from airflow.sdk.definitions.variable import Variable
30+
from airflow.sdk.exceptions import AirflowRuntimeError
2831

2932
if TYPE_CHECKING:
3033
from collections.abc import Callable
@@ -434,3 +437,25 @@ class VariableInterval:
434437
"""
435438

436439
key: str
440+
441+
def resolve(self) -> timedelta:
442+
warnings.warn(
443+
"VariableInterval.resolve() is deprecated and will be removed in a future release. "
444+
"Deadline interval resolution is handled internally during deadline evaluation.",
445+
DeprecationWarning,
446+
stacklevel=2,
447+
)
448+
449+
try:
450+
value = Variable.get(self.key)
451+
except AirflowRuntimeError as e:
452+
raise ValueError(f"VariableInterval '{self.key}' not found") from e
453+
454+
try:
455+
seconds = int(value)
456+
except (TypeError, ValueError) as e:
457+
raise ValueError(
458+
f"VariableInterval '{self.key}' must be an integer (seconds), got: {value!r}"
459+
) from e
460+
461+
return timedelta(seconds=seconds)

‎task-sdk/tests/task_sdk/definitions/test_deadline.py‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,15 @@
1717
from __future__ import annotations
1818

1919
from datetime import datetime, timedelta
20+
from unittest import mock
2021

2122
import pytest
2223
from task_sdk.definitions.test_callback import TEST_CALLBACK_KWARGS, TEST_CALLBACK_PATH, UNIMPORTABLE_DOT_PATH
2324

2425
from airflow.sdk.definitions.callback import AsyncCallback, SyncCallback
25-
from airflow.sdk.definitions.deadline import DeadlineAlert, DeadlineReference
26+
from airflow.sdk.definitions.deadline import DeadlineAlert, DeadlineReference, VariableInterval
27+
from airflow.sdk.definitions.variable import Variable
28+
from airflow.sdk.exceptions import AirflowRuntimeError
2629

2730
DAG_ID = "dag_id_1"
2831
RUN_ID = 1
@@ -162,3 +165,51 @@ def test_deadline_alert_rejects_invalid_callback(self):
162165
interval=timedelta(hours=1),
163166
callback="not_a_callback", # type: ignore
164167
)
168+
169+
170+
class TestVariableInterval:
171+
@pytest.mark.parametrize(
172+
("value", "expected"),
173+
[
174+
("3", timedelta(seconds=3)),
175+
("10", timedelta(seconds=10)),
176+
("05", timedelta(seconds=5)),
177+
("0", timedelta(0)),
178+
("-5", timedelta(seconds=-5)),
179+
],
180+
)
181+
def test_resolve_valid(self, mocker, value, expected):
182+
mocker.patch.object(Variable, "get", return_value=value)
183+
184+
interval = VariableInterval(key="test_interval")
185+
186+
with pytest.warns(DeprecationWarning, match="VariableInterval.resolve"):
187+
assert interval.resolve() == expected
188+
189+
@pytest.mark.parametrize(
190+
("value", "raise_runtime", "match"),
191+
[
192+
(None, True, "not found"),
193+
("abc", False, "must be an integer"),
194+
("", False, "must be an integer"),
195+
],
196+
)
197+
def test_resolve_invalid(self, mocker, value, raise_runtime, match):
198+
if raise_runtime:
199+
mock_err = mock.Mock()
200+
mock_err.error.value = "MISSING"
201+
mock_err.detail = "missing"
202+
203+
mocker.patch.object(
204+
Variable,
205+
"get",
206+
side_effect=AirflowRuntimeError(mock_err),
207+
)
208+
else:
209+
mocker.patch.object(Variable, "get", return_value=value)
210+
211+
interval = VariableInterval(key="test_interval")
212+
213+
with pytest.warns(DeprecationWarning, match="VariableInterval.resolve"):
214+
with pytest.raises(ValueError, match=match):
215+
interval.resolve()

0 commit comments

Comments
 (0)