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

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

Closed
bbovenzi opened this issue Nov 21, 2024 · 2 comments
Closed

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

bbovenzi opened this issue Nov 21, 2024 · 2 comments
Labels
area:UI Related to UI/UX. For Frontend Developers.

Comments

@bbovenzi
Copy link
Contributor

bbovenzi commented Nov 21, 2024

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.

@bbovenzi bbovenzi converted this from a draft issue Nov 21, 2024
@bbovenzi bbovenzi changed the title AIP-38 | Add show_trigger_form_if_no_params to Dag Trigger Form AIP-38 | Add show_trigger_form_if_no_params to Dag Trigger Nov 21, 2024
@bbovenzi bbovenzi changed the title AIP-38 | Add show_trigger_form_if_no_params to Dag Trigger AIP-38 | Add show_trigger_form_if_no_params config to Dag Trigger Nov 21, 2024
@dosubot dosubot bot added the area:UI Related to UI/UX. For Frontend Developers. label Nov 21, 2024
@jscheffl
Copy link
Contributor

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
Copy link
Contributor Author

Works for me! Let me add this to the list of config to remove before 3.0 is released #43519

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.
Projects
Development

No branches or pull requests

2 participants