search

apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
37.26k stars 14.33k forks source link

AIP-38 | Add show_trigger_form_if_no_params config to Dag Trigger #44256

Closed bbovenzi closed 15 hours ago

bbovenzi commented 20 hours ago

We need to add a check for the config value of show_trigger_form_if_no_params. One problem is that right now the Dags list doesn't say if a dag has params or not so we don't know if we need to show the modal or not.

Additionally, I wonder if this should be overridden if require_confirmation_dag_change is True.

jscheffl commented 15 hours ago

Answering the question here:

I'd propose to DROP this legacy UI parameter. It is from ancient times - and the method of triggering is anyway different in the new UI.

In the past we had the UI showing the JSON. Then AIP-50 was implemented which rendered the UI.,. or skipped.

But in the new UI anyway the modal will be displayed. Why? Because I assume the user should confirm the trigger. Should not be a single-click to start a DAG "by accident". And the current modal also has the advanced options in the accordeon - no need to have a config option for this.

The only question is if the information about the form parmaeters should be pre-fetched. I propose the follwing approach: When the user opens the modal a request is made to get the DAG params. While the resuest is running a spinner can be displayed above the accordeon. If params are defined then the spinner can be replaced with the form and the modal can grow in size. If no params the spinner could just be hidden.

bbovenzi commented 15 hours ago

Works for me! Let me add this to the list of config to remove before 3.0 is released https://github.com/apache/airflow/issues/43519