-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Feat: Support URL params for pre-filling Trigger and Backfill forms #59231
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
Feat: Support URL params for pre-filling Trigger and Backfill forms #59231
Conversation
jscheffl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Looks clear and good for me.
I tried but the trigger form was not opening. Don't know if I mis-understood but I needed to click the "trigger" button to open the form pre-populated. Otherwise working fine. I'd expected it is oepning pre-populated when calling with a prepared/pre-filled URL.
Hi @jscheffl Thanks for catching that! You are correct, I missed wiring the URL state to the modal visibility logic in the initial commit. I have pushed a fix in the latest commit (Overview.tsx) where I check for the mode parameter in the URL. Now, if the URL contains /trigger/single or /trigger/backfill, the modal will open automatically with the pre-populated values. |
|
Thanks for the rework. I was testing and it seems there is a bit of a glitch because the two-way synchronization of the UI gets broken if not all parameters are passed in the UI trigger URL. For example using http://localhost:28080/dags/example_params_ui_tutorial/trigger/single?number_param=77 is changes the dictionary of the run-conf to the value as passed but the form fields of the trigger form are not updated to be in sync. And even worse, if you click on the form (any element) the JSON dictionary will be back-populated to the form elements and most form elements dis-appear. So a fix is needed to merge/sync the given parameters of the URL to the default values not to break the form and needs to ensure that updated form values are correctly populated to the UI. |
|
Hi @jscheffl Thanks for catching this ! Changes made:
Please let me know if this looks good to you! |
81d9293 to
7536398
Compare
jscheffl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me this looks good and I am happy that the function is back!
In the past there was always some discussion about excessive use of "useEffect()" so I'd leave this PR open - from my side it is good - but another pair of Web Development knowledge would be good to ensure it is implemented as it should be.
Thanks @jscheffl for the approval! I used useEffect to safely sync the async DAG params with URL inputs using react-hook-form's reset() method. This ensures the form updates correctly once data is loaded. That said, I'm happy to refactor if the UI team prefers a different pattern!" |
pierrejeambrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind rebasing and fixing the merge conflict so we can merge it.
Yes we tend to try to avoid useEffect, but sometimes there's no good way to do that and keep the useEffect.
I think it will be fine for now.
7536398 to
ff73706
Compare
|
Please also fix CI errors. |
ff73706 to
168fe87
Compare
|
Thanks for the continued review!
Ready for a final look! |
|
Oh, still small glitches in static checks. Maybe you run them locally before committing. |
168fe87 to
a67f379
Compare
jscheffl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the numerous rework. Did a final test. Looks good for me!
|
Hi @anshuksi282-ksolves! While working on latest main I realized a regression in the UI caused by inconsistent updates. Can you provide a fix? Else I'd propose to revert this PR and re-apply with a proper fix. It seems the state management and bi-directional updates are not working anymore reliably. Steps to re-produce:
(In general it seems no change of the form fields updates the JSON configuration in "Advanced options" section anymore and thus any change in the form yields to no effect when triggering.) Also one nit: I just realized that the "Advanced options" section is always expanded also if the form just has params defined. I understood the diff before as being a feature if a URL param is passed but now realize that the logic also hits when the UI is just opened. Oh and found another even more error situation, if you have used the dag overview page and search, e.g. Dags for name "params" ...then the URL is changed to something like ...okay sorry, as of writing the bug here and ending up in three errors I'm going to revert now. Can you please re-apply the PR with fixes? |
…pache#59231) * Feat: Support URL params for pre-filling Trigger and Backfill forms * Fix docs build: Correct RST formatting and add 'reprocessed' to wordlist * Fix auto-open modal and update docs * Fix: Sync URL params with defaults and update UI store to prevent data loss * Merge main branch and resolve conflicts * Refactor: Move TriggerDag types and logic to utils to satisfy line limit
…pache#59231) * Feat: Support URL params for pre-filling Trigger and Backfill forms * Fix docs build: Correct RST formatting and add 'reprocessed' to wordlist * Fix auto-open modal and update docs * Fix: Sync URL params with defaults and update UI store to prevent data loss * Merge main branch and resolve conflicts * Refactor: Move TriggerDag types and logic to utils to satisfy line limit


closes: #54800
Description
This PR restores and enhances the ability to pre-populate the Trigger DAG and Backfill forms via URL query parameters in the Airflow 3.0 UI. This feature existed in Airflow 2.x and is useful for sharing pre-configured run links.
Key Changes:
URL Param Parsing:
useSearchParamshook (replacing manual parsing logic from stale PRs).getTriggerConfto handle both JSON strings (?conf={"a":1}) and Key-Value pairs (?a=1).Form Updates:
run_id,logical_date,note, andconf.from_date,to_date,max_active_runs,reprocess_behavior, andrun_backwards.Code Quality:
RunBackfillForm.tsxlogic to keep the file size strictly under the 250-line linting limit.from_date/to_date) to match the API and UI components.Documentation:
docs/core-concepts/params.rstto document the supported URL parameters and examples.How to reproduce / Test
You can test this by running the UI and navigating to any DAG (e.g.,
tutorial) using the following URLs:1. Trigger Single Run (JSON Config):
The modal should open, fields should be filled, and the JSON editor should show the config.
http://localhost:8080/dags/tutorial/trigger/single?run_id=test_run_01¬e=Testing_Note&conf={"environment":"dev","debug":true}
2. Trigger Single Run (Key-Value Config):
The "Advanced Options" should open and show
{"foo": "bar"}.http://localhost:8080/dags/tutorial/trigger/single?foo=bar&retry=3
3. Backfill Form:
The Backfill form should open with dates, max runs, and the "Run Backwards" checkbox pre-selected.
http://localhost:8080/dags/tutorial/trigger/backfill?from_date=2024-01-01T00:00:00&to_date=2024-01-05T00:00:00&max_active_runs=5&run_backwards=true
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.