kunwp1 commented on code in PR #8551:
URL: https://github.com/apache/texera/pull/8551#discussion_r4043086475
##########
frontend/src/app/workspace/service/execute-workflow/execute-workflow.service.ts:
##########
@@ -241,6 +267,17 @@ export class ExecuteWorkflowService {
const selectedUnit =
this.computingUnitStatusService.getSelectedComputingUnitValue();
const computingUnitId = selectedUnit?.computingUnit.cuid;
+ // The warehouse this execution writes to (#7817); undefined serializes
away,
+ // which the backend today reads as the shared default storage (#7751
+ // tightens that to a rejection while the feature is enabled).
+ const warehouseId = this.warehouseService.getSelectedWarehouseIdValue();
+
+ // Final belt for callers that reach this method directly; the public entry
+ // points refuse BEFORE they reset the previous execution's state.
+ if (this.refuseToRunWithoutWarehouse()) {
+ return;
+ }
Review Comment:
I think this caller is not reachable in production (Only from the spec file)
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts:
##########
@@ -409,6 +497,109 @@ export class ComputingUnitSelectionComponent implements
OnInit {
}
}
+ /**
+ * Fetches the warehouse list, on init and on every dropdown open (mirroring
+ * onDropdownVisibilityChange). Preselection re-runs only when the current
+ * pick is gone (first load, or the picked warehouse was deleted), so a
+ * routine refresh cannot override a manual pick.
+ */
+ private refreshWarehouses(): void {
+ this.warehouseRefreshRequested$.next();
+ }
+
+ /**
+ * Mirrors the CU preselection for warehouses (#7817): pick the latest
+ * execution's warehouse when it still exists, else the user's first
+ * warehouse — so a run needs no explicit pick.
+ */
+ private applyWarehousePreselect(): void {
+ if (
+ this.manualWarehousePick &&
+ this.selectedWarehouseId !== undefined &&
+ this.warehouses.some(warehouse => warehouse.whid ===
this.selectedWarehouseId)
+ ) {
+ return;
+ }
+ if (!this.warehouseEnabled || this.warehouses.length === 0) {
+ // Nothing selectable: drop any pick the root-scoped service still
holds, so a
+ // stale id cannot ride the next execution while the picker stays hidden.
+ this.warehouseService.selectWarehouse(undefined);
+ return;
+ }
+ const lastUsed = this.warehouses.find(warehouse => warehouse.whid ===
this.lastExecutionWhid);
+ this.warehouseService.selectWarehouse((lastUsed ??
this.warehouses[0]).whid);
+ }
+
+ /** The warehouse the workflow last ran against, for the preselect;
first-warehouse fallback otherwise. */
+ private preselectWarehouseFromLatestExecution(wid: number): void {
+ const stillShown = () => wid === this.workflowId;
+ this.workflowExecutionsService
+ .retrieveLatestWorkflowExecution(wid)
+ .pipe(untilDestroyed(this))
+ .subscribe({
+ next: (latestWorkflowExecution: WorkflowExecutionsEntry) => {
+ if (stillShown()) {
+ this.lastExecutionWhid = latestWorkflowExecution.whId ?? undefined;
+ this.applyWarehousePreselect();
+ }
+ },
+ error: () => {
+ if (stillShown()) {
+ this.applyWarehousePreselect();
+ }
+ },
+ });
+ }
+
+ onWarehouseSelected(whid: number): void {
+ this.manualWarehousePick = true;
+ this.warehouseService.selectWarehouse(whid);
+ }
+
+ public trackByWhid(_idx: number, warehouse: DashboardWarehouse): number {
+ return warehouse.whid;
+ }
+
+ onWarehouseDropdownVisibilityChange(visible: boolean): void {
+ if (visible) {
+ this.refreshWarehouses();
+ }
+ }
+
+ get selectedWarehouse(): DashboardWarehouse | undefined {
+ return this.warehouses.find(warehouse => warehouse.whid ===
this.selectedWarehouseId);
+ }
+
+ /**
+ * True when the deployment enables per-user warehouses but none is selected.
+ * The menu's Run button redirects to the create-warehouse modal in this
+ * state, mirroring the computing-unit Connect flow: with the feature on,
+ * every execution must have a warehouse to write to.
+ */
+ get warehouseRequiredButMissing(): boolean {
+ return this.warehouseEnabled && this.selectedWarehouseId === undefined;
+ }
+
+ getWarehouseButtonText(): string {
+ return this.selectedWarehouse?.name ?? "Warehouse";
+ }
+
+ showAddWarehouseModalVisible(): void {
+ this.addWarehouseModalVisible = true;
+ }
+
+ onWarehouseCreated(warehouse: DashboardWarehouse): void {
+ // Mirrors onComputingUnitCreated: a warehouse created from the workspace
is
+ // what the next execution should write to — as explicit a choice as a
pick.
+ this.manualWarehousePick = true;
+ this.warehouseService.selectWarehouse(warehouse.whid);
+ this.refreshWarehouses();
Review Comment:
I don't think we need to do a round-trip to the backend here but instead
update `this.warehouses` locally as what we did for the previous PR.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts:
##########
@@ -409,6 +497,109 @@ export class ComputingUnitSelectionComponent implements
OnInit {
}
}
+ /**
+ * Fetches the warehouse list, on init and on every dropdown open (mirroring
+ * onDropdownVisibilityChange). Preselection re-runs only when the current
+ * pick is gone (first load, or the picked warehouse was deleted), so a
+ * routine refresh cannot override a manual pick.
+ */
+ private refreshWarehouses(): void {
+ this.warehouseRefreshRequested$.next();
+ }
+
+ /**
+ * Mirrors the CU preselection for warehouses (#7817): pick the latest
+ * execution's warehouse when it still exists, else the user's first
+ * warehouse — so a run needs no explicit pick.
+ */
+ private applyWarehousePreselect(): void {
+ if (
+ this.manualWarehousePick &&
+ this.selectedWarehouseId !== undefined &&
+ this.warehouses.some(warehouse => warehouse.whid ===
this.selectedWarehouseId)
+ ) {
+ return;
+ }
+ if (!this.warehouseEnabled || this.warehouses.length === 0) {
+ // Nothing selectable: drop any pick the root-scoped service still
holds, so a
+ // stale id cannot ride the next execution while the picker stays hidden.
+ this.warehouseService.selectWarehouse(undefined);
+ return;
+ }
+ const lastUsed = this.warehouses.find(warehouse => warehouse.whid ===
this.lastExecutionWhid);
+ this.warehouseService.selectWarehouse((lastUsed ??
this.warehouses[0]).whid);
+ }
+
+ /** The warehouse the workflow last ran against, for the preselect;
first-warehouse fallback otherwise. */
+ private preselectWarehouseFromLatestExecution(wid: number): void {
+ const stillShown = () => wid === this.workflowId;
+ this.workflowExecutionsService
+ .retrieveLatestWorkflowExecution(wid)
+ .pipe(untilDestroyed(this))
+ .subscribe({
+ next: (latestWorkflowExecution: WorkflowExecutionsEntry) => {
+ if (stillShown()) {
+ this.lastExecutionWhid = latestWorkflowExecution.whId ?? undefined;
+ this.applyWarehousePreselect();
+ }
+ },
+ error: () => {
+ if (stillShown()) {
+ this.applyWarehousePreselect();
+ }
+ },
+ });
+ }
Review Comment:
Can you refactor this by merging it into `selectFromLastExecution` because I
see a lot of duplicates?
##########
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:
This seems like a valid concern.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts:
##########
@@ -226,6 +256,49 @@ export class ComputingUnitSelectionComponent implements
OnInit {
this.allComputingUnits = units;
});
+ // Warehouse picker state (#7817). The pick itself lives in
WarehouseService,
+ // where ExecuteWorkflowService reads it at execution time.
+ this.warehouseRefreshRequested$
+ .pipe(
+ switchMap(() =>
+ this.warehouseService.getStatus().pipe(
+ // Caught inside the switchMap so a failure ends only this request,
+ // not the stream.
+ catchError((err: unknown) => {
+ // The pick lives in the root-scoped service, so clearing the
list
+ // is not enough: a stale id would still ride the next execution
+ // request. The flag falls back to the boot-time config instead
of
+ // false — failing open here would un-gate Run on an enabled
+ // deployment just because one status request failed.
+ this.warehouseEnabled = this.config.env.warehouseEnabled;
+ this.warehouses = [];
+ this.warehouseService.selectWarehouse(undefined);
+ console.error("Failed to fetch warehouse status", err);
+ return EMPTY;
+ })
+ )
+ ),
+ untilDestroyed(this)
+ )
+ .subscribe(status => {
+ this.warehouseEnabled = status.enabled;
+ this.warehouses = [...status.warehouses];
+ if (
+ this.selectedWarehouseId === undefined ||
+ !this.warehouses.some(warehouse => warehouse.whid ===
this.selectedWarehouseId)
Review Comment:
I think some of the conditions overlap with the one in
`applyWarehousePreselect` which makes the conditions unreachable.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts:
##########
@@ -226,6 +256,49 @@ export class ComputingUnitSelectionComponent implements
OnInit {
this.allComputingUnits = units;
});
+ // Warehouse picker state (#7817). The pick itself lives in
WarehouseService,
+ // where ExecuteWorkflowService reads it at execution time.
+ this.warehouseRefreshRequested$
+ .pipe(
+ switchMap(() =>
+ this.warehouseService.getStatus().pipe(
+ // Caught inside the switchMap so a failure ends only this request,
+ // not the stream.
+ catchError((err: unknown) => {
+ // The pick lives in the root-scoped service, so clearing the
list
+ // is not enough: a stale id would still ride the next execution
+ // request. The flag falls back to the boot-time config instead
of
+ // false — failing open here would un-gate Run on an enabled
+ // deployment just because one status request failed.
+ this.warehouseEnabled = this.config.env.warehouseEnabled;
+ this.warehouses = [];
+ this.warehouseService.selectWarehouse(undefined);
Review Comment:
Add a notificationservice error message as well to tell the user if the
error occurred. Also, instead of clearing up the whole selection list, clear
only on an authoritative answer like a successful response with `enabled:
false` or a successful list that no longer contains the pick.
--
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]
