Repository navigation
fix(frontend): fall back to a running unit on workflow load - #8935
Conversation
selectFromLastExecution now waits for the loaded unit list and selects the last execution's unit only while it is still listed, otherwise the first Running unit, and the no-history fallback waits for the same list instead of reading it synchronously.
Automated Reviewer SuggestionsBased on the
|
Backport auto-label reportThis
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8935 +/- ##
============================================
- Coverage 91.95% 91.82% -0.14%
+ Complexity 5081 5073 -8
============================================
Files 1280 1274 -6
Lines 55622 55415 -207
Branches 7048 7016 -32
============================================
- Hits 51147 50884 -263
- Misses 2733 2808 +75
+ Partials 1742 1723 -19
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/request-review @kunwp1 |
kunwp1
left a comment
There was a problem hiding this comment.
Thanks, the fix looks right to me.
The fallback in selectFromLoadedUnits now skips units without WRITE access, since Run is disabled on them, and the wait for the first loaded unit list is shared with the remembered-unit branch through a new onceUnitsLoaded helper.
|
This PR is blocked by the release labels. @mengw15 Should we remove the release labels? |
|
@eugenegujing Can you address the frontend unit test failure? |
|
@kunwp1 PTAL! |
What changes were proposed in this PR?
Root cause. When a workflow opens,
ComputingUnitSelectionComponent.selectFromLastExecutionselects the unit the workflow last ran on without checking that the unit is still listed./api/executions/{wid}/latestalso returns terminated units, soComputingUnitStatusService.selectComputingUnitwaits forever for acuidthat never appears, and the workflow opens with no unit selected even while another unit is Running. The "first Running unit" fallback only ran when the lookup failed, and it read the unit list synchronously, so it could select nothing if the list had not loaded yet.Both screenshots: the workflow last ran on "Analytics Unit", which was then terminated, and the workflow was reloaded. "My Computing Unit" is Running.
Change. Both branches of
selectFromLastExecutionnow go through a newselectFromLoadedUnits, which waits for the first non-empty unit list, asselectInitialUnitalready does for a remembered unit. It then selects the last execution's unit if it is listed, else the first Running unit the user has WRITE access to, else nothing. The unit list includes units shared with READ access, and the menu disables Run on those, so falling back to one would select a unit the workflow cannot run on; the fallback skips them. The last execution's unit is not access-filtered: the workflow ran on it before, so it is restored as is. The wait for the loaded list is shared between the remembered-unit branch and the new fallback through a smallonceUnitsLoadedhelper, which also carries the guard that drops a decision once the workflow has changed underneath it. The warehouse preselect is unchanged.Not in scope:
Any related issues, documentation, discussions?
Closes #8933
The last-execution path dates from #3400. The remembered-unit path, which already checks the list, came with #8456.
How was this PR tested?
New tests in
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.spec.ts:makeComputingUnitin the spec now takes anaccessPrivilegeoverride, defaulting to WRITE, so the existing fixtures are unchanged.Existing tests in the same
describethat go through the latest-execution path now pass their unit list tobootWithMetaStream(units), since the choice depends on the loaded list. Their expectations are unchanged. "prefers the remembered unit for this workflow over the latest execution" now also puts the latest execution's unit in the list; otherwise the new code would skip that unit anyway, and the test would no longer show that the remembered unit wins. The existing remembered-unit tests, including "drops a pending remembered decision once the workflow has changed underneath it", now go through the shared helper and pass unchanged.On Node 25 the spec run also needs
NODE_OPTIONS="--no-experimental-webstorage", since its built-inlocalStorageshadows jsdom's. CI runs Node 24 withyarn --cwd frontend run test:ciand needs no flag.Mutation check: each of these makes at least one new test fail: dropping the list check, skipping a latest execution that has no unit, picking the last Running unit instead of the first, picking the first unit regardless of status, accepting any unit that is not Pending, dropping the WRITE check from the fallback, access-filtering the last execution's unit as well, dropping
untilDestroyed, dropping the workflow-change guard in the shared helper (caught from both the remembered and the latest-execution path), dropping the non-empty-list wait, droppingtake(1), reading the list synchronously in the no-history branch, ignoringselectUnit = false, and preferring a Running unit over the last execution's unit.Manual check in Chrome, 1512x861 viewport, against a build of this branch on the local-dev stack with local computing units:
Also checked by hand: when the last execution's unit is still listed, that unit is selected rather than the first Running one; a workflow that has never run selects the first Running unit.
Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (claude-fable-5-1, claude-opus-5-5). Co-authored with Claude; the author reviewed the change before submission.