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

show_trigger_form_if_no_params allows to trigger without params even when params required #37379

Closed
2 tasks done
kmehkeri opened this issue Feb 13, 2024 · 4 comments
Closed
2 tasks done
Labels
area:UI Related to UI/UX. For Frontend Developers. duplicate Issue that is duplicated kind:bug This is a clearly a bug

Comments

@kmehkeri
Copy link

Apache Airflow version

2.8.1

If "Other Airflow 2 version" selected, which one?

No response

What happened?

For show_trigger_form_if_no_params=False (default) the trigger form is only shown when any params are present.

I wanted to have an option to override logical date for a paramless DAG, so I set show_trigger_form_if_no_params=True. However now, instead of just showing the form, clicking on trigger displays a small combo "Trigger DAG" / "Trigger DAG w/ config".

What you think should happen instead?

Trigger form should be always shown. Just as the option says.

How to reproduce

Set how_trigger_form_if_no_params=False in configuration. Trigger DAG from UI.

Operating System

Using official Airflow Docker image in standalone mode

Versions of Apache Airflow Providers

Standard as of official Docker image.

Deployment

Official Apache Airflow Helm Chart

Deployment details

No response

Anything else?

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@kmehkeri kmehkeri added area:core kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Feb 13, 2024
@kmehkeri
Copy link
Author

kmehkeri commented Feb 13, 2024

Hm... I just realized this is a reverse of #33392.

If a trigger form was shown for DAGs that have params it was important for me and my users to get exactly same experience with paramless DAGs - even if there are no params they can just have a quick look and confirm triggering. And sometimes I want to override logical date or run id.

However having that selection: with or without config... they're confused. They trigger a DAG that requires params without params just because it was the first choice in the combo. It's way better to just show the form where it's clear what params are needed, if any.

I will surely educate users, but I would still argue about at least having third option for this setting. I would be happy to contribute if you agree it's a good way to proceed. The only concern I could have is switching the option from simple boolean to three-state.

@Taragolis
Copy link
Contributor

If I understand correctly than #37063 should resolve which should be part of Airflow 2.8.2.

I've check in main, if you try to trigger DAGRun without the parameters and parameters are mandatory, then it redirect to form.

@Taragolis
Copy link
Contributor

I will surely educate users, but I would still argue about at least having third option for this setting. I would be happy to contribute if you agree it's a good way to proceed. The only concern I could have is switching the option from simple boolean to three-state.

If you see how it could be improved feel free to raise a PR, (see Contributions Docs). Discussion around some implementation would be better rather than abstract.

@jscheffl
Copy link
Contributor

Yes, agree to @Taragolis - this seems to be 99% a duplicate of #36843 which is fixed in 2.8.2.

Also take note that the logical date and run_id fiel now has moved into the lower "generated configuration" section as it was confusing most users.

If you want more or specific things, then current functions cal always be extended via a contribution if it is generic and can be used by others as well. This is how the form feature initially was introduced.

@jscheffl jscheffl added duplicate Issue that is duplicated area:UI Related to UI/UX. For Frontend Developers. and removed area:core needs-triage label for new issues that we didn't triage yet labels Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers. duplicate Issue that is duplicated kind:bug This is a clearly a bug
Projects
None yet
Development

No branches or pull requests

3 participants