You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by et...@apache.org on 2025/02/22 07:16:53 UTC
(iceberg) branch 1.8.x updated: Core: Remove namespace/table/view HEAD endpoints from defaults (#12351) (#12368)
This is an automated email from the ASF dual-hosted git repository.
etudenhoefner pushed a commit to branch 1.8.x
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/1.8.x by this push:
new 3c6bf24a86 Core: Remove namespace/table/view HEAD endpoints from defaults (#12351) (#12368)
3c6bf24a86 is described below
commit 3c6bf24a86d005895a3fdb95d820a4019990fefb
Author: Eduard Tudenhoefner <et...@gmail.com>
AuthorDate: Sat Feb 22 08:16:47 2025 +0100
Core: Remove namespace/table/view HEAD endpoints from defaults (#12351) (#12368)
---
.../apache/iceberg/rest/RESTSessionCatalog.java | 7 ++--
.../org/apache/iceberg/rest/TestRESTCatalog.java | 46 +++++++++++++++-------
.../apache/iceberg/rest/TestRESTViewCatalog.java | 29 +++++++++-----
open-api/rest-catalog-open-api.yaml | 4 --
4 files changed, 56 insertions(+), 30 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
index 1cde2b8ad4..32fb400804 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
@@ -135,17 +135,17 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
.addAll(TOKEN_PREFERENCE_ORDER)
.build();
+ // these default endpoints must not be updated in order to maintain backwards compatibility with
+ // legacy servers
private static final Set<Endpoint> DEFAULT_ENDPOINTS =
ImmutableSet.<Endpoint>builder()
.add(Endpoint.V1_LIST_NAMESPACES)
.add(Endpoint.V1_LOAD_NAMESPACE)
- .add(Endpoint.V1_NAMESPACE_EXISTS)
.add(Endpoint.V1_CREATE_NAMESPACE)
.add(Endpoint.V1_UPDATE_NAMESPACE)
.add(Endpoint.V1_DELETE_NAMESPACE)
.add(Endpoint.V1_LIST_TABLES)
.add(Endpoint.V1_LOAD_TABLE)
- .add(Endpoint.V1_TABLE_EXISTS)
.add(Endpoint.V1_CREATE_TABLE)
.add(Endpoint.V1_UPDATE_TABLE)
.add(Endpoint.V1_DELETE_TABLE)
@@ -155,11 +155,12 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
.add(Endpoint.V1_COMMIT_TRANSACTION)
.build();
+ // these view endpoints must not be updated in order to maintain backwards compatibility with
+ // legacy servers
private static final Set<Endpoint> VIEW_ENDPOINTS =
ImmutableSet.<Endpoint>builder()
.add(Endpoint.V1_LIST_VIEWS)
.add(Endpoint.V1_LOAD_VIEW)
- .add(Endpoint.V1_VIEW_EXISTS)
.add(Endpoint.V1_CREATE_VIEW)
.add(Endpoint.V1_UPDATE_VIEW)
.add(Endpoint.V1_DELETE_VIEW)
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
index 696240bb6d..0455e2cf0d 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
@@ -2508,6 +2508,15 @@ public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
@Test
public void testNamespaceExistsFallbackToGETRequest() {
+ // server indicates support of loading a namespace only via GET, which is
+ // what older REST servers would send back too
+ verifyNamespaceExistsFallbackToGETRequest(
+ ConfigResponse.builder()
+ .withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_NAMESPACE))
+ .build());
+ }
+
+ private void verifyNamespaceExistsFallbackToGETRequest(ConfigResponse configResponse) {
RESTCatalogAdapter adapter =
Mockito.spy(
new RESTCatalogAdapter(backendCatalog) {
@@ -2518,13 +2527,7 @@ public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
Consumer<ErrorResponse> errorHandler,
Consumer<Map<String, String>> responseHeaders) {
if ("v1/config".equals(request.path())) {
- return castResponse(
- responseType,
- ConfigResponse.builder()
- // server indicates support of loading a namespace only via GET, which is
- // what older REST servers would send back too
- .withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_NAMESPACE))
- .build());
+ return castResponse(responseType, configResponse);
}
return super.execute(request, responseType, errorHandler, responseHeaders);
@@ -2553,6 +2556,13 @@ public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
any());
}
+ @Test
+ public void testNamespaceExistsFallbackToGETRequestWithLegacyServer() {
+ // simulate a legacy server that doesn't send back supported endpoints, thus the
+ // client relies on the default endpoints
+ verifyNamespaceExistsFallbackToGETRequest(ConfigResponse.builder().build());
+ }
+
@Test
public void testTableExistsViaHEADRequest() {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
@@ -2578,6 +2588,13 @@ public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
@Test
public void testTableExistsFallbackToGETRequest() {
+ // server indicates support of loading a table only via GET, which is
+ // what older REST servers would send back too
+ verifyTableExistsFallbackToGETRequest(
+ ConfigResponse.builder().withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_TABLE)).build());
+ }
+
+ private void verifyTableExistsFallbackToGETRequest(ConfigResponse configResponse) {
RESTCatalogAdapter adapter =
Mockito.spy(
new RESTCatalogAdapter(backendCatalog) {
@@ -2588,13 +2605,7 @@ public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
Consumer<ErrorResponse> errorHandler,
Consumer<Map<String, String>> responseHeaders) {
if ("v1/config".equals(request.path())) {
- return castResponse(
- responseType,
- ConfigResponse.builder()
- // server indicates support of loading a table only via GET, which is
- // what older REST servers would send back too
- .withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_TABLE))
- .build());
+ return castResponse(responseType, configResponse);
}
return super.execute(request, responseType, errorHandler, responseHeaders);
@@ -2627,6 +2638,13 @@ public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
any());
}
+ @Test
+ public void testTableExistsFallbackToGETRequestWithLegacyServer() {
+ // simulate a legacy server that doesn't send back supported endpoints, thus the
+ // client relies on the default endpoints
+ verifyTableExistsFallbackToGETRequest(ConfigResponse.builder().build());
+ }
+
private RESTCatalog catalog(RESTCatalogAdapter adapter) {
RESTCatalog catalog =
new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config) -> adapter);
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java
index 73ff5f052a..0214e8762a 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java
@@ -246,6 +246,15 @@ public class TestRESTViewCatalog extends ViewCatalogTests<RESTCatalog> {
@Test
public void viewExistsFallbackToGETRequest() {
+ // server indicates support of loading a view only via GET, which is
+ // what older REST servers would send back too
+ verifyViewExistsFallbackToGETRequest(
+ ConfigResponse.builder().withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_VIEW)).build(),
+ ImmutableMap.of());
+ }
+
+ private void verifyViewExistsFallbackToGETRequest(
+ ConfigResponse configResponse, Map<String, String> catalogProperties) {
RESTCatalogAdapter adapter =
Mockito.spy(
new RESTCatalogAdapter(backendCatalog) {
@@ -256,13 +265,7 @@ public class TestRESTViewCatalog extends ViewCatalogTests<RESTCatalog> {
Consumer<ErrorResponse> errorHandler,
Consumer<Map<String, String>> responseHeaders) {
if ("v1/config".equals(request.path())) {
- return castResponse(
- responseType,
- ConfigResponse.builder()
- // server indicates support of loading a view only via GET, which is
- // what older REST servers would send back too
- .withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_VIEW))
- .build());
+ return castResponse(responseType, configResponse);
}
return super.execute(request, responseType, errorHandler, responseHeaders);
@@ -271,8 +274,7 @@ public class TestRESTViewCatalog extends ViewCatalogTests<RESTCatalog> {
RESTCatalog catalog =
new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config) -> adapter);
- catalog.initialize("test", ImmutableMap.of());
-
+ catalog.initialize("test", catalogProperties);
assertThat(catalog.viewExists(TableIdentifier.of("ns", "view"))).isFalse();
Mockito.verify(adapter)
@@ -289,6 +291,15 @@ public class TestRESTViewCatalog extends ViewCatalogTests<RESTCatalog> {
any());
}
+ @Test
+ public void viewExistsFallbackToGETRequestWithLegacyServer() {
+ // simulate a legacy server that doesn't send back supported endpoints, thus the
+ // client relies on the default endpoints
+ verifyViewExistsFallbackToGETRequest(
+ ConfigResponse.builder().build(),
+ ImmutableMap.of(RESTSessionCatalog.VIEW_ENDPOINTS_SUPPORTED, "true"));
+ }
+
@Override
protected RESTCatalog catalog() {
return restCatalog;
diff --git a/open-api/rest-catalog-open-api.yaml b/open-api/rest-catalog-open-api.yaml
index 4b82fb3a3b..446adfe4f8 100644
--- a/open-api/rest-catalog-open-api.yaml
+++ b/open-api/rest-catalog-open-api.yaml
@@ -111,8 +111,6 @@ paths:
- GET /v1/{prefix}/namespaces/{namespace}
- - HEAD /v1/{prefix}/namespaces/{namespace}
-
- DELETE /v1/{prefix}/namespaces/{namespace}
- POST /v1/{prefix}/namespaces/{namespace}/properties
@@ -123,8 +121,6 @@ paths:
- GET /v1/{prefix}/namespaces/{namespace}/tables/{table}
- - HEAD /v1/{prefix}/namespaces/{namespace}/tables/{table}
-
- POST /v1/{prefix}/namespaces/{namespace}/tables/{table}
- DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}