From a50136dd0f0e5a486191fc97cbffa8c6530e13a4 Mon Sep 17 00:00:00 2001 From: Antonio Date: Mon, 23 Mar 2026 13:28:49 -0300 Subject: [PATCH 1/3] fix(providers/standard): remove premature param value validation in HITLOperator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HITLOperator params are form fields filled by a human at runtime. Calling `self.params.validate()` in `__init__` incorrectly validates param values at DAG parse time, before any human input exists. This causes `ParamValidationError` for any param without an explicit default value, breaking DAG import. The fix removes `self.params.validate()` from `validate_params()`. Value validation (type, required fields, schema) already happens correctly in `validate_params_input()` after the human submits the form. The `_options` key check is preserved — it is a structural constraint, not a value check. Closes #59551 Co-Authored-By: Claude Opus 4.6 --- .../providers/standard/operators/hitl.py | 9 ++- .../unit/standard/operators/test_hitl.py | 74 +++++++++++++------ 2 files changed, 59 insertions(+), 24 deletions(-) diff --git a/providers/standard/src/airflow/providers/standard/operators/hitl.py b/providers/standard/src/airflow/providers/standard/operators/hitl.py index d3199a7b0a14b..c4c1be95649cb 100644 --- a/providers/standard/src/airflow/providers/standard/operators/hitl.py +++ b/providers/standard/src/airflow/providers/standard/operators/hitl.py @@ -147,10 +147,15 @@ def validate_params(self) -> None: """ Validate the `params` attribute of the instance. + Note: Value validation (e.g., required fields, schema) is intentionally skipped here + because HITLOperator params represent form fields that are filled by a human at runtime. + Values do not exist at DAG parse time, so validating them in ``__init__`` would cause + a ``ParamValidationError`` for any param without a default. Value validation happens + in ``validate_params_input`` after the human submits the form. + Raises: - ValueError: If `"_options"` key is present in `params`, which is not allowed. + ValueError: If ``"_options"`` key is present in ``params``, which is not allowed. """ - self.params.validate() if "_options" in self.params: raise ValueError('"_options" is not allowed in params') diff --git a/providers/standard/tests/unit/standard/operators/test_hitl.py b/providers/standard/tests/unit/standard/operators/test_hitl.py index e1ebae5ab00ef..3221889b44a15 100644 --- a/providers/standard/tests/unit/standard/operators/test_hitl.py +++ b/providers/standard/tests/unit/standard/operators/test_hitl.py @@ -139,27 +139,9 @@ def test_validate_options_with_empty_options(self) -> None: params=ParamsDict({"input_1": 1}), ) - @pytest.mark.parametrize( - ("params", "exc", "error_msg"), - ( - (ParamsDict({"_options": 1}), ValueError, '"_options" is not allowed in params'), - ( - ParamsDict({"param": Param("", type="integer")}), - ParamValidationError, - ( - "Invalid input for param param: '' is not of type 'integer'\n\n" - "Failed validating 'type' in schema:\n" - " {'type': 'integer'}\n\n" - "On instance:\n ''" - ), - ), - ), - ) - def test_validate_params( - self, params: ParamsDict, exc: type[ValueError | ParamValidationError], error_msg: str - ) -> None: - # validate_params is called during initialization - with pytest.raises(exc, match=error_msg): + def test_validate_params_rejects_options_key(self) -> None: + """_options is a reserved key and must not be allowed in params.""" + with pytest.raises(ValueError, match='"_options" is not allowed in params'): HITLOperator( task_id="hitl_test", subject="This is subject", @@ -167,9 +149,57 @@ def test_validate_params( body="This is body", defaults=["1"], multiple=False, - params=params, + params=ParamsDict({"_options": 1}), ) + def test_param_without_default_does_not_raise_on_init(self) -> None: + """Regression test for #59551. + + HITLOperator params are form fields filled by a human at runtime. A param with no + default value is valid — the human provides the value when submitting the form. + Validating param values in __init__ incorrectly raises ParamValidationError at DAG + parse time, before any value exists. + """ + # Must not raise ParamValidationError + op = HITLOperator( + task_id="hitl_test", + subject="This is subject", + options=["1", "2"], + body="This is body", + defaults=["1"], + multiple=False, + params={"my_param": Param(type="string")}, + ) + assert "my_param" in op.params + + def test_param_with_default_does_not_raise_on_init(self) -> None: + """Params with explicit defaults continue to work normally.""" + op = HITLOperator( + task_id="hitl_test", + subject="This is subject", + options=["1", "2"], + body="This is body", + defaults=["1"], + multiple=False, + params={"my_param": Param("hello", type="string")}, + ) + assert "my_param" in op.params + + def test_param_with_wrong_value_type_does_not_raise_on_init(self) -> None: + """Value validation is deferred to validate_params_input at runtime, not __init__.""" + # Param("", type="integer") has a value that doesn't match the schema, but + # this must NOT raise at init time — the human will provide the correct value at runtime. + op = HITLOperator( + task_id="hitl_test", + subject="This is subject", + options=["1", "2"], + body="This is body", + defaults=["1"], + multiple=False, + params={"param": Param("", type="integer")}, + ) + assert "param" in op.params + def test_validate_defaults(self) -> None: hitl_op = HITLOperator( task_id="hitl_test", From c3f1af0fd5c3a2fb712920c37034a55166861358 Mon Sep 17 00:00:00 2001 From: Antonio Date: Mon, 23 Mar 2026 23:42:22 -0300 Subject: [PATCH 2/3] Add newsfragment for HITLOperator ParamValidationError fix --- providers/standard/newsfragments/64108.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 providers/standard/newsfragments/64108.bugfix.rst diff --git a/providers/standard/newsfragments/64108.bugfix.rst b/providers/standard/newsfragments/64108.bugfix.rst new file mode 100644 index 0000000000000..11e161818a56f --- /dev/null +++ b/providers/standard/newsfragments/64108.bugfix.rst @@ -0,0 +1 @@ +Fix HITLOperator ``ParamValidationError`` when params lack default values at DAG import time From 9856d5bd4eff608e7dc4be25fabf808621aad89b Mon Sep 17 00:00:00 2001 From: Antonio Date: Tue, 24 Mar 2026 09:08:45 -0300 Subject: [PATCH 3/3] Address review feedback - Remove newsfragment (not needed per reviewer) - Consolidate param validation tests into @pytest.mark.parametrize - Fix docstring: DAG -> Dag for consistency --- .../standard/newsfragments/64108.bugfix.rst | 1 - .../providers/standard/operators/hitl.py | 2 +- .../unit/standard/operators/test_hitl.py | 62 ++++++++----------- 3 files changed, 27 insertions(+), 38 deletions(-) delete mode 100644 providers/standard/newsfragments/64108.bugfix.rst diff --git a/providers/standard/newsfragments/64108.bugfix.rst b/providers/standard/newsfragments/64108.bugfix.rst deleted file mode 100644 index 11e161818a56f..0000000000000 --- a/providers/standard/newsfragments/64108.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Fix HITLOperator ``ParamValidationError`` when params lack default values at DAG import time diff --git a/providers/standard/src/airflow/providers/standard/operators/hitl.py b/providers/standard/src/airflow/providers/standard/operators/hitl.py index c4c1be95649cb..bef40d504a3fc 100644 --- a/providers/standard/src/airflow/providers/standard/operators/hitl.py +++ b/providers/standard/src/airflow/providers/standard/operators/hitl.py @@ -149,7 +149,7 @@ def validate_params(self) -> None: Note: Value validation (e.g., required fields, schema) is intentionally skipped here because HITLOperator params represent form fields that are filled by a human at runtime. - Values do not exist at DAG parse time, so validating them in ``__init__`` would cause + Values do not exist at Dag parse time, so validating them in ``__init__`` would cause a ``ParamValidationError`` for any param without a default. Value validation happens in ``validate_params_input`` after the human submits the form. diff --git a/providers/standard/tests/unit/standard/operators/test_hitl.py b/providers/standard/tests/unit/standard/operators/test_hitl.py index 3221889b44a15..fb6839e0967c4 100644 --- a/providers/standard/tests/unit/standard/operators/test_hitl.py +++ b/providers/standard/tests/unit/standard/operators/test_hitl.py @@ -152,28 +152,33 @@ def test_validate_params_rejects_options_key(self) -> None: params=ParamsDict({"_options": 1}), ) - def test_param_without_default_does_not_raise_on_init(self) -> None: + @pytest.mark.parametrize( + ("params", "expected_key"), + [ + pytest.param( + {"my_param": Param(type="string")}, + "my_param", + id="no_default", + ), + pytest.param( + {"my_param": Param("hello", type="string")}, + "my_param", + id="with_default", + ), + pytest.param( + {"param": Param("", type="integer")}, + "param", + id="wrong_value_type", + ), + ], + ) + def test_param_value_validation_deferred_to_runtime(self, params: dict, expected_key: str) -> None: """Regression test for #59551. - HITLOperator params are form fields filled by a human at runtime. A param with no - default value is valid — the human provides the value when submitting the form. - Validating param values in __init__ incorrectly raises ParamValidationError at DAG - parse time, before any value exists. + HITLOperator params are form fields filled by a human at runtime. + Value validation (required, schema) must NOT happen in __init__ — it is + deferred to ``validate_params_input`` after the human submits the form. """ - # Must not raise ParamValidationError - op = HITLOperator( - task_id="hitl_test", - subject="This is subject", - options=["1", "2"], - body="This is body", - defaults=["1"], - multiple=False, - params={"my_param": Param(type="string")}, - ) - assert "my_param" in op.params - - def test_param_with_default_does_not_raise_on_init(self) -> None: - """Params with explicit defaults continue to work normally.""" op = HITLOperator( task_id="hitl_test", subject="This is subject", @@ -181,24 +186,9 @@ def test_param_with_default_does_not_raise_on_init(self) -> None: body="This is body", defaults=["1"], multiple=False, - params={"my_param": Param("hello", type="string")}, - ) - assert "my_param" in op.params - - def test_param_with_wrong_value_type_does_not_raise_on_init(self) -> None: - """Value validation is deferred to validate_params_input at runtime, not __init__.""" - # Param("", type="integer") has a value that doesn't match the schema, but - # this must NOT raise at init time — the human will provide the correct value at runtime. - op = HITLOperator( - task_id="hitl_test", - subject="This is subject", - options=["1", "2"], - body="This is body", - defaults=["1"], - multiple=False, - params={"param": Param("", type="integer")}, + params=params, ) - assert "param" in op.params + assert expected_key in op.params def test_validate_defaults(self) -> None: hitl_op = HITLOperator(