Fix trigger DAG form submitting stale config when reusing a previous run#65360
Open
seruman wants to merge 3 commits intoapache:mainfrom
Open
Fix trigger DAG form submitting stale config when reusing a previous run#65360seruman wants to merge 3 commits intoapache:mainfrom
seruman wants to merge 3 commits intoapache:mainfrom
Conversation
When retriggering a DAG with prefilled config from a previous run, edits made via the Run Parameters form fields were not synced back to react-hook-form's conf field. The useEffect that syncs the Zustand store conf to the form skipped updates when prefillConfig was set, so the submitted conf was always the original prefilled value. Remove the !prefillConfig guard so the sync fires regardless of how the dialog was opened. Add tests covering two-way sync between form fields and the JSON editor.
seruman
requested review from
bbovenzi,
choo121600,
guan404ming,
pierrejeambrun,
ryanahamilton,
shubhamraj-git and
vatsrahul1001
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When using Trigger again with this config, edits made to params via the Run Parameters form fields are silently discarded on submit. The form sends the original prefilled values instead. Editing the JSON directly in Advanced Options works fine, only form field edits are lost.
The
useEffectthat propagates Zustand store changes back to react-hook-form had a!prefillConfigguard, added in #56406 alongside the prefill feature. My guess is this was meant to prevent the sync effect from overwriting the prefilled values with default conf before the prefill effect had a chance to run. Makes sense as a precaution, but it also blocks all subsequent store updates like user edits from reaching the form'sconffield. 1Removing the guard means the sync effect now also fires during the initial prefill, right after the prefill effect calls
setConf. This should be a no-op2 since both effects end up with the same conf value, the prefill effect does a fullreset(...)and thensetConf(confString), and the sync effect picks up that sameconfStringand doesreset(prev => ({...prev, conf}))which changes nothing.So the initial render looks fine, and subsequent edits now should propagate correctly.
closes: #65357
Was generative AI tooling used to co-author this PR?
Generated-by: pi (Claude Opus 4.6) following the guidelines
Note
I was already suspecting a state mismatch, used it to find the relevant code and confirm the hypothesis. Rest of the component changes are on me. Also had a little help mocking out the stuff like Monaco editor and the i18n stuff.
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.Footnotes
I might be missing the real reason behind the guard but on my tests removing did not introduced any issues. ↩
🤞 ↩