Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Params Definition is Required to be JSON Schema Valid also if Manually Triggered #31299

Closed
2 tasks done
jscheffl opened this issue May 15, 2023 · 1 comment · Fixed by #31301
Closed
2 tasks done

Params Definition is Required to be JSON Schema Valid also if Manually Triggered #31299

jscheffl opened this issue May 15, 2023 · 1 comment · Fixed by #31301
Labels

Comments

@jscheffl
Copy link
Contributor

Apache Airflow version

2.6.0

What happened

While trying to debug other things I stumbled over the function DAG.validate_schedule_and_params() which is used in the constructor of the DAG class. This function validates all DAG params for JSON Schema correctness - but only if the trigger condition is not set to manual. From point of code I understood that this is made especially to have a valid DAG but validate params at time of triggering.

I tried to apply this as a "feature" in the Trigger UI Forms from AIP-50 and realized that DAG parsing in dagbag.py:444 calls dag.validate() which still does a full validation. So the DAG can not be rendered if params are not valid against JSON Schema.

What you think should happen instead

So if we have a manual triggered DAG it would be cool to relax param validation and check on submission. This would allow to define required fields and enforce a user to prove content w/o having a default and the user just hits "Trigger".

Besides this it makes the constructor and the dag.validate() call consistent in regards of checks.

How to reproduce

Run your Airflow instance and edit airflow/example_dags/example_params_ui_tutorial.py in line 108 for example and set the default value of required_field to None. You will get a DAG parsing error even the DAG is manually triggered.

Operating System

Ubuntu 20.04

Versions of Apache Airflow Providers

defaults

Deployment

Official Apache Airflow Helm Chart

Deployment details

Standard Docker deployment of Airflow 2.6.0

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@jscheffl jscheffl added area:core kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels May 15, 2023
@jscheffl jscheffl changed the title Params Definition is Required to ve JSON Schema Valid also if Manually Triggered Params Definition is Required to be JSON Schema Valid also if Manually Triggered May 15, 2023
@eladkal eladkal added good first issue affected_version:2.6 Issues Reported for 2.6 and removed needs-triage label for new issues that we didn't triage yet labels May 16, 2023
@ricardogaspar2
Copy link

Yeah we much needed a required like field to relax the new params
Thanks for doing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants