mengw15 commented on code in PR #6049:
URL: https://github.com/apache/texera/pull/6049#discussion_r3509751868
##########
.github/workflows/build.yml:
##########
@@ -281,6 +281,8 @@ jobs:
# those run in the amber-integration job below.
env:
AMBER_TEST_FILTER: skip-integration
+ # unit job uses its provisioned postgres catalog; default (rest)
needs a Lakekeeper not run here
+ STORAGE_ICEBERG_CATALOG_TYPE: postgres
Review Comment:
Thanks. #6045 moved the specs that test the catalog (IcebergDocumentSpec
etc. — direct getInstance()); those are @IntegrationTest now.
But this PR surfaced more: amber's "unit" job also runs non-unit tests —
engine e2e specs (+ one scheduling) that run a full workflow and materialize
results into Iceberg as a side effect, so they need the default catalog.
Verified with rest/no-Lakekeeper, these fail: DataProcessingSpec, PauseSpec,
ReconfigurationSpec, DefaultCostEstimatorSpec — they only passed before because
the default was postgres (provisioned by the unit job).
I'll mark this draft while we settle that; the postgres pin just keeps CI
green for now. Two directions:
(a) treat them as integration (@IntegrationTest → amber-integration): + unit
job truly catalog-free; − moves core engine tests to the slower job, and a
scheduling spec doesn't obviously belong in "integration".
(b) decouple the result store
Either is its own refactor PR. WDYT?
##########
common/config/src/main/resources/storage.conf:
##########
@@ -21,7 +21,7 @@ storage {
# Configuration for Apache Iceberg, used for storing the workflow results
& stats
iceberg {
catalog {
- type = postgres # either hadoop, rest, or postgres
+ type = rest # either hadoop, rest, or postgres
Review Comment:
I think so — hadoop maps to a HadoopCatalog on the local filesystem. But
I've never used the hadoop catalog and don't have much context on it. I tried
it for the unit job here, but HadoopCatalog floods commit-conflict errors under
amber's concurrent result writes.
FWIW it looks like the earliest catalog type — added with the original
IcebergDocument, before rest/postgres were introduced — I don't see it used in
any deployment/CI. So it may be effectively legacy.
--
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]
