jscheffl commented on PR #43367: URL: https://github.com/apache/airflow/pull/43367#issuecomment-2463327713
> > Looking promising. Since there is a conflict... let me know when it is best to do a re-review. > > I noticed a few details: > > > > * Trigger button on DAG details is still disabled, would be great to add the modal there as well > > * Trigger is not really working, correct? I did not see a successful run later > > * Still the dialog has three lines, title, DAG name and DAG id. I thing this can really be reduced, e.g. put the DAG ID into the title and the gray DAG ID only needs to be displayed if no title is there. > > * If the DAG is disabled - on the legacy UI there was an option added and turned on per default to enable DAG scheduling if the DAG was disabled before opening the trigger dialog > > * Default params of the DAG are not loaded per default into the DICT element > > 1. Added the button on DAG details too. > > 2. As discussed , using an alert and console, Can be taken in another PR post fast API migration. > > 3. Did the changes, It will be like "Trigger DAG - <DAG_DISPLAY_NAME>" and if it's not present, DAG id will be shown on next line. > > 4. Can we take this in another PR? Since that needs a validation check. > > 5. You mean conf? > > > @jscheffl Regarding 4) Can you leave a TODO as comment. Then we can leave this in a follow-up PR Regarding 5) Yes. Default params --> config dict on the modal dialog --> trigger to be the DAG run conf. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
