Ma77Ball commented on code in PR #8551:
URL: https://github.com/apache/texera/pull/8551#discussion_r4031586652
##########
frontend/src/app/workspace/component/menu/menu.component.ts:
##########
@@ -408,6 +421,18 @@ export class MenuComponent implements OnInit, OnDestroy {
};
}
+ // Per-user warehouses enabled but none to write to (#7817): mirror the
+ // Connect state above — name the fixing action, and runWorkflow() routes
+ // the click into the create-warehouse modal.
+ if (this.computingUnitSelectionComponent?.warehouseRequiredButMissing) {
Review Comment:
The warehouse gate sits before the `executionState` switch, so it replaces
the button for every state, including Running and Paused.
Why it matters: with the feature enabled while a workflow is Running (or
Paused), deleting the selected warehouse from the picker clears the pick
(`applyWarehousePreselect` -> `selectWarehouse(undefined)` when the last one is
gone), `warehouseRequiredButMissing` flips true, and this branch returns
"Create Warehouse" instead of Pause/Kill - the user loses control of the
in-flight execution from the primary button. The gate is only meaningful in the
states that would start a run.
Suggested fix:
```suggestion
if (
this.computingUnitSelectionComponent?.warehouseRequiredButMissing &&
[
ExecutionState.Uninitialized,
ExecutionState.Completed,
ExecutionState.Terminated,
ExecutionState.Killed,
ExecutionState.Failed,
].includes(this.executionState)
) {
```
##########
frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.ts:
##########
@@ -230,6 +240,22 @@ export class ExecuteWorkflowService {
);
}
+ /**
+ * While the deployment requires a warehouse (#7817) and none is picked,
+ * refuses with a toast and returns true. Checked at every public entry
+ * point before it resets the previous execution's state — a refused click
+ * must not wipe the results already on screen — and again in
+ * sendExecutionRequest as the shared belt (#7751 adds the backend-side
+ * rejection).
+ */
+ private refuseToRunWithoutWarehouse(): boolean {
+ if (!this.config.env.warehouseEnabled ||
this.warehouseService.getSelectedWarehouseIdValue() !== undefined) {
Review Comment:
This gate keys off `this.config.env.warehouseEnabled` (the boot-time GUI
config), while the picker component keys `warehouseEnabled` off the
`/warehouse/status` response's `enabled`.
Why it matters: the two are coupled only because both trace back to
`StorageConfig.warehouseEnabled` server-side today, so they agree in practice
(hence P3, not a bug). But they are two independent client signals for one
gate: if `status.enabled` ever becomes dynamic or per-user, this gate could
refuse a run ("Create or select a warehouse before running.") while the picker
is hidden, stranding the user. Consider reading the same signal in both places
(e.g. deriving the gate from `WarehouseService` status) so they cannot drift.
--
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]
