Iceberg REST Phase 1 [3/3]: Tests and CI#500
Open
cbb330 wants to merge 4 commits intolinkedin:mainfrom
Open
Conversation
Vendor the upstream Apache Iceberg REST OpenAPI spec (v1.10) and wire Polaris-aligned codegen into the services/tables Gradle build: - spec/iceberg-rest-catalog-open-api.yaml: upstream spec as source of truth - Gradle tasks: setUpOpenApiCliForIcebergRest, validateIcebergRestOpenApiSpec, generateIcebergRestOpenApiServer with importMappings/typeMappings to map spec schemas to real Iceberg library types (no model generation) - Post-processing for Iceberg 1.10-only types mapped to Object (compatibility with our 1.5.2 fork) - compileJava depends on codegen; check depends on spec validation - jackson-databind-nullable dependency for generated code
cbb330
force-pushed
the
chbush/iceberg-rest-3-tests
branch
4 times, most recently
from
f2190c2 to
8d2e93f
Compare
- Strip non-upstream comments (CODE_COPIED_TO_POLARIS, version marker) so the vendored spec is byte-for-byte identical to apache-iceberg-1.10.0 - Add verifyIcebergRestSpecSync Gradle task that downloads the upstream spec from the pinned tag and fails if it differs from the vendored copy - Wire verifyIcebergRestSpecSync into the check task so CI catches drift
cbb330
force-pushed
the
chbush/iceberg-rest-3-tests
branch
from
8d2e93f to
4e8030c
Compare
Implement the read-only Iceberg REST Catalog surface on top of the
generated OpenAPI interfaces from PR1:
Controller (IcebergRestCatalogController):
- Implements CatalogApiApi + ConfigurationApiApi generated interfaces
- GET /v1/config returns prefix override for route isolation
- GET /v1/{prefix}/namespaces/{ns}/tables lists tables via TablesService
- GET /v1/{prefix}/namespaces/{ns}/tables/{t} loads table via CatalogHandlers
- HEAD /v1/{prefix}/namespaces/{ns}/tables/{t} checks table existence
- All unimplemented endpoints return 501 via generated defaults
Serialization:
- IcebergRestHttpMessageConverter for Iceberg REST types (kebab-case JSON)
- IcebergRestSerde with Iceberg RESTSerializers + kebab-case ObjectMapper
- IcebergRestSerdeConfig registers converter without affecting existing Jackson
Error handling:
- IcebergRestExceptionHandler scoped to controller, returns Iceberg ErrorResponse
Service:
- TablesService.searchTables(databaseId, actingPrincipal) overload for auth
Comprehensive test coverage for the Iceberg REST Catalog endpoints: Round-trip integration tests (IcebergRestCatalogRoundTripTest, 21 tests): - List tables: single namespace, correct namespace, cross-namespace isolation, empty namespace - Load table: schema columns, field IDs, required fields, different schemas, partition spec, unpartitioned, location, properties, sort order, snapshot, cross-namespace - Table exists: HEAD for existing, missing, wrong namespace - Error handling: load nonexistent table, load from nonexistent namespace - Consistency: multiple loads return same metadata Unit tests (IcebergRestCatalogControllerTest, 6 tests): - MockMvc tests for config, list, load, load not found, head exists, head not found CI: - PyIceberg smoke test (iceberg_rest_catalog_smoke.py) validates stock RESTCatalog client against live service - build-run-tests.yml adds :services:tables:check and smoke test step
cbb330
force-pushed
the
chbush/iceberg-rest-3-tests
branch
from
4e8030c to
4896bad
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part 3 of 3 for Iceberg REST Catalog support. Adds comprehensive test coverage and CI for the controller from #499.
Changes
Tests
Round-trip integration tests (
IcebergRestCatalogRoundTripTest, 21 tests):RESTCatalogclient against a Spring Boot test serverNoSuchTableException), load from nonexistent namespaceUnit tests (
IcebergRestCatalogControllerTest, 6 tests):CI:
iceberg_rest_catalog_smoke.py— PyIceberg smoke test validates stockRestCatalogclient against live service (table_exists, list_tables, load_table)build-run-tests.yml— adds:services:tables:checkto Gradle build and PyIceberg smoke test stepReview guidance
This PR's new files (623 lines across 4 files):
IcebergRestCatalogRoundTripTest.java(380 lines)IcebergRestCatalogControllerTest.java(150 lines)iceberg_rest_catalog_smoke.py(82 lines)build-run-tests.yml(+11 lines)The diff against
mainalso includes #498 and #499's files — those are reviewed in their respective PRs.Testing Done
All 27 tests pass locally:
Manually tested with PyIceberg and DuckDB against local Docker setup.
Additional Information
PR Stack (merge in order)