Skip to content

fix(providers/standard): remove premature param value validation in HITLOperator#64108

Merged
Lee-W merged 3 commits intoapache:mainfrom
antonio-mello-ai:fix/hitl-param-no-default
Mar 26, 2026
Merged

fix(providers/standard): remove premature param value validation in HITLOperator#64108
Lee-W merged 3 commits intoapache:mainfrom
antonio-mello-ai:fix/hitl-param-no-default

Conversation

@antonio-mello-ai
Copy link
Contributor

Summary

Fixes a regression introduced when HITLOperator.validate_params() was extended to call self.params.validate(). HITL params are form fields filled by a human at runtime — validating their values at DAG parse time (__init__) causes ParamValidationError for any param that lacks an explicit default value, breaking DAG import.

Root cause: HITLOperator.__init__validate_params()self.params.validate()Param.resolve() raises ParamValidationError("No value passed and Param has no default value") if the param has no default. This validation runs at parse time, but no human input exists yet at that point.

Fix: Remove self.params.validate() from validate_params(). Value validation (type, schema, required fields) already happens correctly in validate_params_input() after the human submits the form.

The "_options" key check is preserved — it is a structural constraint that can and should be validated at init time.

Credit to @henry3260 who identified the same fix in #59653, which closed as stale without a review.

Before fixing

# DAG import fails with:
# ParamValidationError: Invalid input for param my_param: No value passed and Param has no default value
HITLOperator(
    task_id="ask_user",
    subject="Please provide input",
    options=["OK"],
    params={"my_param": Param(type="string")},  # no default — valid HITL use case
)

After fixing

# DAG imports successfully. Value is validated when the human submits the form.
HITLOperator(
    task_id="ask_user",
    subject="Please provide input",
    options=["OK"],
    params={"my_param": Param(type="string")},
)

Changes

  • providers/standard/src/airflow/providers/standard/operators/hitl.py: removed self.params.validate() from validate_params(); added a docstring clarifying why value validation is intentionally deferred
  • providers/standard/tests/unit/standard/operators/test_hitl.py:

Closes #59551


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

Are there any user-facing changes?

Yes — DAGs using HITLOperator (or any subclass) with params that have no explicit default value will now import successfully, as they should.

Are there any breaking changes?

No. The removed call self.params.validate() was incorrect behavior — it caused ParamValidationError at DAG parse time. Removing it restores correct behavior without impacting any valid use case.

Commit message for squash merge:

fix(providers/standard): remove premature param value validation in HITLOperator

Generated with AI assistance: Implemented with Claude Code. The logic, root cause analysis, and test strategy were reviewed and confirmed correct.

@henry3260
Copy link
Contributor

Thank you for taking this up!

Copy link
Contributor

@Shrividya Shrividya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix, thanks for taking this on! The root cause explanation in the PR description is really clear.

@antonio-mello-ai antonio-mello-ai force-pushed the fix/hitl-param-no-default branch from 3301678 to d1ff14b Compare March 24, 2026 02:43
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix, LGTM overall.

@Lee-W
Copy link
Member

Lee-W commented Mar 24, 2026

overall looks good

@antonio-mello-ai antonio-mello-ai force-pushed the fix/hitl-param-no-default branch from d1ff14b to 2030b0c Compare March 24, 2026 12:10
@antonio-mello-ai
Copy link
Contributor Author

Thanks for the review! All three items addressed:

  1. Removed the newsfragment
  2. Consolidated the three param validation tests into a single @pytest.mark.parametrize
  3. Fixed docstring: DAG -> Dag

@Lee-W Lee-W force-pushed the fix/hitl-param-no-default branch from 2030b0c to cd4f82f Compare March 24, 2026 14:10
@Lee-W Lee-W requested a review from jason810496 March 25, 2026 10:35
antonio-mello-ai and others added 3 commits March 26, 2026 12:28
…ITLOperator

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 apache#59551

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove newsfragment (not needed per reviewer)
- Consolidate param validation tests into @pytest.mark.parametrize
- Fix docstring: DAG -> Dag for consistency
@Lee-W Lee-W force-pushed the fix/hitl-param-no-default branch from cd4f82f to 9856d5b Compare March 26, 2026 04:28
@Lee-W Lee-W merged commit fbe7cbe into apache:main Mar 26, 2026
279 of 280 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: DAG import fails with HITLOperator when params have no explicit default (ParamValidationError)

5 participants