Skip to content

Commit cd4f82f

Browse files
antonio-mello-aiLee-W
authored andcommitted
Address review feedback
- Remove newsfragment (not needed per reviewer) - Consolidate param validation tests into @pytest.mark.parametrize - Fix docstring: DAG -> Dag for consistency
1 parent e029661 commit cd4f82f

3 files changed

Lines changed: 27 additions & 38 deletions

File tree

providers/standard/newsfragments/64108.bugfix.rst

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

providers/standard/src/airflow/providers/standard/operators/hitl.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ def validate_params(self) -> None:
149149
150150
Note: Value validation (e.g., required fields, schema) is intentionally skipped here
151151
because HITLOperator params represent form fields that are filled by a human at runtime.
152-
Values do not exist at DAG parse time, so validating them in ``__init__`` would cause
152+
Values do not exist at Dag parse time, so validating them in ``__init__`` would cause
153153
a ``ParamValidationError`` for any param without a default. Value validation happens
154154
in ``validate_params_input`` after the human submits the form.
155155

providers/standard/tests/unit/standard/operators/test_hitl.py

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -152,53 +152,43 @@ def test_validate_params_rejects_options_key(self) -> None:
152152
params=ParamsDict({"_options": 1}),
153153
)
154154

155-
def test_param_without_default_does_not_raise_on_init(self) -> None:
155+
@pytest.mark.parametrize(
156+
("params", "expected_key"),
157+
[
158+
pytest.param(
159+
{"my_param": Param(type="string")},
160+
"my_param",
161+
id="no_default",
162+
),
163+
pytest.param(
164+
{"my_param": Param("hello", type="string")},
165+
"my_param",
166+
id="with_default",
167+
),
168+
pytest.param(
169+
{"param": Param("", type="integer")},
170+
"param",
171+
id="wrong_value_type",
172+
),
173+
],
174+
)
175+
def test_param_value_validation_deferred_to_runtime(self, params: dict, expected_key: str) -> None:
156176
"""Regression test for #59551.
157177
158-
HITLOperator params are form fields filled by a human at runtime. A param with no
159-
default value is valid — the human provides the value when submitting the form.
160-
Validating param values in __init__ incorrectly raises ParamValidationError at DAG
161-
parse time, before any value exists.
178+
HITLOperator params are form fields filled by a human at runtime.
179+
Value validation (required, schema) must NOT happen in __init__ — it is
180+
deferred to ``validate_params_input`` after the human submits the form.
162181
"""
163-
# Must not raise ParamValidationError
164-
op = HITLOperator(
165-
task_id="hitl_test",
166-
subject="This is subject",
167-
options=["1", "2"],
168-
body="This is body",
169-
defaults=["1"],
170-
multiple=False,
171-
params={"my_param": Param(type="string")},
172-
)
173-
assert "my_param" in op.params
174-
175-
def test_param_with_default_does_not_raise_on_init(self) -> None:
176-
"""Params with explicit defaults continue to work normally."""
177182
op = HITLOperator(
178183
task_id="hitl_test",
179184
subject="This is subject",
180185
options=["1", "2"],
181186
body="This is body",
182187
defaults=["1"],
183188
multiple=False,
184-
params={"my_param": Param("hello", type="string")},
185-
)
186-
assert "my_param" in op.params
187-
188-
def test_param_with_wrong_value_type_does_not_raise_on_init(self) -> None:
189-
"""Value validation is deferred to validate_params_input at runtime, not __init__."""
190-
# Param("", type="integer") has a value that doesn't match the schema, but
191-
# this must NOT raise at init time — the human will provide the correct value at runtime.
192-
op = HITLOperator(
193-
task_id="hitl_test",
194-
subject="This is subject",
195-
options=["1", "2"],
196-
body="This is body",
197-
defaults=["1"],
198-
multiple=False,
199-
params={"param": Param("", type="integer")},
189+
params=params,
200190
)
201-
assert "param" in op.params
191+
assert expected_key in op.params
202192

203193
def test_validate_defaults(self) -> None:
204194
hitl_op = HITLOperator(

0 commit comments

Comments
 (0)