This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8713-3afdeb0b492b832eeab1b0787820b80d117bfdf4 in repository https://gitbox.apache.org/repos/asf/texera.git
commit f30af7e7714fae1f7d55b2c1d4b14c13592329f5 Author: Meng Wang <[email protected]> AuthorDate: Mon Sep 28 21:12:36 2026 +0000 feat: turn on per-user warehouses (#8713) ### What changes were proposed in this PR? Flips `storage.warehouse.enabled` to `true`, enabling the per-user warehouse feature that has been built behind it. All of #6870's Phase 0 work is merged; the switch is the last step. The comment above it is updated too — it described a feature still being built. The switch only takes effect where the Iceberg catalog is `rest` (Lakekeeper) — `StorageConfig` keeps the feature off for any other catalog type, so deployments that do not run Lakekeeper are unaffected. ### Any related issues, documentation, discussions? Closes #8712. Part of #6870. Proposed and demonstrated in discussion #8633, with no objections raised. ### How was this PR tested? - Ran the engine's integration suite locally against a live Lakekeeper and MinIO, with the switch on and off. The same eight cases fail either way (Python UDF specs timing out in my environment), so the switch makes no difference to the suite. The `amber` unit job pins `STORAGE_ICEBERG_CATALOG_TYPE: postgres`, which keeps the feature off there regardless of this flag. - Exercised the feature by hand on a local deployment with the switch on: creating a warehouse from the dashboard tab and from the canvas picker, running a workflow into it, and confirming that a run with no warehouse selected is refused rather than routed to shared storage. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-5, claude-fable-5) --- .../texera/web/service/ExecutionResultServiceSpec.scala | 13 ++++++++----- .../texera/web/service/ResultExportServiceSpec.scala | 12 +++++++----- common/config/src/main/resources/storage.conf | 7 ++++--- .../apache/texera/common/config/StorageConfigSpec.scala | 17 ++++++++++++----- 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala b/amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala index 73c69886ec..3d9ed9fcf8 100644 --- a/amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala @@ -950,13 +950,16 @@ class ExecutionResultServiceSpec thrown.getMessage shouldBe "No execution is recorded" } - it should "refuse to read a result stored in a per-user warehouse while the feature is off" in { + it should "refuse to read a result whose warehouse the URI cannot resolve" in { val executionId = newExecution() // Only the URI row is needed: the guard has to reject before the document is - // opened, and no table was ever created for this warehouse. The guard reads - // StorageConfig.warehouseEnabled, which ships (and runs in CI) as false; - // WarehouseReadGuardSpec pins both settings of the flag directly. - insertResultUri(executionId, resultUriOf(executionId, "erss-warehouse", Some("byo"))) + // opened, and no table was ever created for this warehouse. An unresolvable + // /wh/ prefix is what the guard refuses in *either* flag state, so this pins + // the wiring — that paginate consults the guard at all — without depending on + // the shipping default. WarehouseReadGuardSpec pins the enabled and disabled + // behaviour of the guard itself. + val resolvable = resultUriOf(executionId, "erss-warehouse", Some("byo")) + insertResultUri(executionId, new URI(resolvable.toString.replace("/wh/byo/", "/wh/a%2Fb/"))) a[WarehouseUnavailableException] should be thrownBy paginate( paginationRequest.copy(operatorID = "erss-warehouse") diff --git a/amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala b/amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala index b53b112096..f19f66f46b 100644 --- a/amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala @@ -1137,12 +1137,14 @@ class ResultExportServiceSpec doc.get().map(_.getField[String]("name")).toList shouldBe List("a", "b") } - it should "refuse a result stored in a per-user warehouse while the feature is off" in { + it should "refuse a result whose warehouse the URI cannot resolve" in { val execution = insertExecution() // No table is needed: the WarehouseReadGuard check (#6930) fires on the URI, - // before any catalog access. warehouseEnabled ships (and runs in CI) as - // false; WarehouseReadGuardSpec pins both settings of the flag directly. - insertResultUri(execution.getEid, resultUriOf(execution.getEid, "rexs-wh", Some("byo"))) + // before any catalog access. An unresolvable /wh/ prefix is refused in either + // flag state, so this pins the wiring rather than the shipping default; + // WarehouseReadGuardSpec pins the enabled and disabled behaviour itself. + val resolvable = resultUriOf(execution.getEid, "rexs-wh", Some("byo")) + insertResultUri(execution.getEid, resolvable.toString.replace("/wh/byo/", "/wh/a%2Fb/")) val ex = intercept[WarehouseUnavailableException] { exportService invokePrivate getOperatorDocument( @@ -1150,7 +1152,7 @@ class ResultExportServiceSpec testComputingUnit.getCuid.intValue() ) } - ex.getMessage should include("warehouse 'byo'") + ex.getMessage should include("unresolvable warehouse URI") } // -- generateFileName -------------------------------------------------------- diff --git a/common/config/src/main/resources/storage.conf b/common/config/src/main/resources/storage.conf index f563c3529a..d8e7e19921 100644 --- a/common/config/src/main/resources/storage.conf +++ b/common/config/src/main/resources/storage.conf @@ -152,10 +152,11 @@ storage { interval-minutes = ${?STORAGE_CLEANUP_INTERVAL_MINUTES} } - # Per-user warehouse feature (#6870). One switch that hides the whole feature until it - # is fully built: merging the warehouse PRs changes nothing while this stays off. + # Per-user warehouse feature (#6870). Only takes effect with the REST catalog + # (Lakekeeper), since warehouses are Lakekeeper entities; any other catalog type + # keeps the feature off whatever this says. warehouse { - enabled = false + enabled = true enabled = ${?STORAGE_WAREHOUSE_ENABLED} } diff --git a/common/config/src/test/scala/org/apache/texera/common/config/StorageConfigSpec.scala b/common/config/src/test/scala/org/apache/texera/common/config/StorageConfigSpec.scala index 45b628a319..4c3fef9ef0 100644 --- a/common/config/src/test/scala/org/apache/texera/common/config/StorageConfigSpec.scala +++ b/common/config/src/test/scala/org/apache/texera/common/config/StorageConfigSpec.scala @@ -19,6 +19,7 @@ package org.apache.texera.common.config +import com.typesafe.config.ConfigFactory import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -50,12 +51,18 @@ class StorageConfigSpec extends AnyFlatSpec with Matchers { StorageConfig.ENV_CLEANUP_INTERVAL_MINUTES shouldBe "STORAGE_CLEANUP_INTERVAL_MINUTES" } - "StorageConfig warehouse settings" should "default the feature to disabled so merging changes nothing" in { - // storage.warehouse.enabled is the kill switch for the whole per-user warehouse - // feature (#6870); this guards the safe default from silently flipping to true. - // Only assert when the env override is unset (e.g. in CI), since it would win otherwise. + "StorageConfig warehouse settings" should "be on by default, and only with the REST catalog" in { + // Only assert when the env override is unset, since it would win otherwise. if (sys.env.get(StorageConfig.ENV_WAREHOUSE_ENABLED).isEmpty) { - StorageConfig.warehouseEnabled shouldBe false + // The resource is read directly as well because CI's amber job pins the + // catalog to postgres, which makes warehouseEnabled false there either + // way and would let a revert of the default pass unnoticed. + ConfigFactory + .parseResources("storage.conf") + .resolve() + .getBoolean("storage.warehouse.enabled") shouldBe true + + StorageConfig.warehouseEnabled shouldBe (StorageConfig.icebergCatalogType == "rest") } }
