Nessie: Support APIv2 client #22215

ajantha-bhat · 2024-05-31T13:42:35Z

Description

By default, Nessie API version will be inferred from Nessie URI. If the User has a custom URI which doesn't have version info in the URI, user can configure iceberg.nessie-catalog.client-api-version

Additional context and related issues

Changes at Iceberg repo: apache/iceberg#9459

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( x) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

ajantha-bhat · 2024-05-31T13:43:13Z

cc: @dimas-b

docs/src/main/sphinx/object-storage/metastores.md

ajantha-bhat · 2024-05-31T13:47:18Z

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogModule.java

                icebergNessieCatalogConfig.getDefaultReferenceName(),
                null,
                ImmutableMap.of());
    }
+
+    private static String inferVersionFromURI(String uri)


Ideally we should have exposed this from Iceberg repo
https://github.com/apache/iceberg/blob/2722290a7793dcb4f411aad271d5a22697c696f9/nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java#L136

But that needs another release (1.6.0, which may take a while)

Return ClientApiVersion.forName(). This will throw if the API is unknown

Didn't use ClientApiVersion.valueOf() but returning the ClientApiVersion from inferVersionFromURI

dimas-b

Nice improvement. LGTM. Thanks, @ajantha-bhat !

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogConfig.java

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogModule.java

ajantha-bhat · 2024-06-04T15:15:14Z

Test failure is because of existing unrelated flaky test: #18697

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogModule.java

ajantha-bhat · 2024-06-06T10:21:45Z

The failure in ci / pt (default, suite-hive-transactional, is unrelated to the change.

wendigo

LGTM % comments

wendigo · 2024-06-06T11:44:33Z

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogModule.java

@@ -58,9 +63,46 @@ public static NessieIcebergClient createNessieIcebergClient(IcebergNessieCatalog
        icebergNessieCatalogConfig.getBearerToken()
                .ifPresent(token -> builder.withAuthentication(BearerAuthenticationProvider.create(token)));

-        return new NessieIcebergClient(builder.build(NessieApiV1.class),
+        // default version is inferred by uri.
+        IcebergNessieCatalogConfig.ClientApiVersion apiVersion = icebergNessieCatalogConfig.getClientAPIVersion()


let's push this logic to IcebergNessieCatalogConfig so that getClientAPIVersion always return a ClientApiVersion:

public ClientApiVersion getClientAPIVersion() { return clientAPIVersion.orElseGet(this::inferVersionFromURI); }

Move inferVersionFromURI to IcebergNessieCatalogConfig

TestIcebergNessieCatalogConfig.testExplicitPropertyMapping is failing because assertAttributesNotEqual(metadata, actual, defaultInstance); will try to get the getClientAPIVersion() using getter method of the default instance (without URI and get the exception).

I think clubbing inferVersionFromURI in the getter is not a good idea. I am planning to call it separately but still keep the inferVersionFromURI method in this class as protected method.

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogModule.java

wendigo · 2024-06-12T14:54:43Z

docs/src/main/sphinx/object-storage/metastores.md

@@ -566,12 +566,15 @@ properties:
 * - `iceberg.nessie-catalog.authentication.token`
  - The token to use with `BEARER` authentication. Example:
 `SXVLUXUhIExFQ0tFUiEK`
+* - `iceberg.nessie-catalog.client-api-version`
+  - Optional client api version to be used instead of inferring from server URI.
+    Available value is `V1` or `V2`.


Available values are

@mosabua ptal for documentation changes

wendigo · 2024-06-12T14:55:23Z

docs/src/main/sphinx/object-storage/metastores.md

@@ -566,12 +566,15 @@ properties:
 * - `iceberg.nessie-catalog.authentication.token`
  - The token to use with `BEARER` authentication. Example:
 `SXVLUXUhIExFQ0tFUiEK`
+* - `iceberg.nessie-catalog.client-api-version`
+  - Optional client api version to be used instead of inferring from server URI.


Client API version to be used. By default API version is inferred from the iceberg.nessie-catalog.uri

wendigo · 2024-06-12T14:55:39Z

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogConfig.java

+    }
+
+    @Config("iceberg.nessie-catalog.client-api-version")
+    @ConfigDescription("Optional client api version to be used instead of inferring from server URI")


Client API version to use

mosabua · 2024-06-12T15:23:58Z

docs/src/main/sphinx/object-storage/metastores.md

@@ -566,12 +566,15 @@ properties:
 * - `iceberg.nessie-catalog.authentication.token`
  - The token to use with `BEARER` authentication. Example:
 `SXVLUXUhIExFQ0tFUiEK`
+* - `iceberg.nessie-catalog.client-api-version`
+  - Client API version to be used. By default API version is inferred from the `iceberg.nessie-catalog.uri`.


Suggested change

- Client API version to be used. By default API version is inferred from the `iceberg.nessie-catalog.uri`.

- Optional version of the Client API version to use. By default it is inferred from the `iceberg.nessie-catalog.uri` value.

mosabua · 2024-06-12T15:24:17Z

docs/src/main/sphinx/object-storage/metastores.md

@@ -566,12 +566,15 @@ properties:
 * - `iceberg.nessie-catalog.authentication.token`
  - The token to use with `BEARER` authentication. Example:
 `SXVLUXUhIExFQ0tFUiEK`
+* - `iceberg.nessie-catalog.client-api-version`
+  - Client API version to be used. By default API version is inferred from the `iceberg.nessie-catalog.uri`.
+    Available values are `V1` or `V2`.


Suggested change

Available values are `V1` or `V2`.

Valid values are `V1` or `V2`.

By default, Nessie API version will be inferred from Nessie URI. If the User has a custom URI which doesn't have version info in the URI, user can configure `iceberg.nessie-catalog.client-api-version`

JunseoChoJJ · 2024-06-13T04:13:56Z

@findepi can we use api/v2 now?

for Iceberg connector

additionalCatalogs:
  nessie_catalog: |-
    connector.name=iceberg
    iceberg.catalog.type=nessie
    iceberg.nessie-catalog.uri=http://host_ip:32327/api/v1
    iceberg.nessie-catalog.ref=main

Is it possible to change catalog.uri to http://10.163.20.222:32327/api/v2?

still got this error on dbeaver when trying to config with api/v2

ajantha-bhat · 2024-06-13T05:48:10Z

@JunseoChoJJ: Once the next version of Trino is released, we can use it.
Which version of Trino did you use?

JunseoChoJJ · 2024-06-14T00:25:34Z

@ajantha-bhat I used trino 449

ajantha-bhat · 2024-06-14T05:42:17Z

upcoming release (450) will have this change.

cla-bot bot added the cla-signed label May 31, 2024

github-actions bot added docs iceberg Iceberg connector labels May 31, 2024

ajantha-bhat commented May 31, 2024

View reviewed changes

docs/src/main/sphinx/object-storage/metastores.md Show resolved Hide resolved

ajantha-bhat commented May 31, 2024

View reviewed changes

ajantha-bhat marked this pull request as draft May 31, 2024 15:08

ajantha-bhat force-pushed the v2 branch 2 times, most recently from 7fb292a to a12a160 Compare June 1, 2024 08:40

ajantha-bhat marked this pull request as ready for review June 3, 2024 04:51

dimas-b approved these changes Jun 3, 2024

View reviewed changes

ajantha-bhat requested a review from wendigo June 3, 2024 13:48

wendigo reviewed Jun 3, 2024

View reviewed changes

ajantha-bhat force-pushed the v2 branch from a12a160 to f94bd72 Compare June 4, 2024 13:03

ajantha-bhat force-pushed the v2 branch from f94bd72 to c30b202 Compare June 5, 2024 13:42

wendigo reviewed Jun 5, 2024

View reviewed changes

...iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/IcebergNessieCatalogModule.java Outdated Show resolved Hide resolved

ajantha-bhat force-pushed the v2 branch from c30b202 to 0475000 Compare June 6, 2024 06:36

wendigo approved these changes Jun 6, 2024

View reviewed changes

ajantha-bhat force-pushed the v2 branch from 0475000 to 5fa0c4d Compare June 6, 2024 13:46

dimas-b approved these changes Jun 6, 2024

View reviewed changes

ajantha-bhat force-pushed the v2 branch from 5fa0c4d to 979f44d Compare June 12, 2024 13:28

wendigo approved these changes Jun 12, 2024

View reviewed changes

wendigo reviewed Jun 12, 2024

View reviewed changes

ajantha-bhat force-pushed the v2 branch from 979f44d to 2e5f32c Compare June 12, 2024 15:21

mosabua reviewed Jun 12, 2024

View reviewed changes

Nessie: Support APIv2 client

3310006

By default, Nessie API version will be inferred from Nessie URI. If the User has a custom URI which doesn't have version info in the URI, user can configure `iceberg.nessie-catalog.client-api-version`

ajantha-bhat force-pushed the v2 branch from 2e5f32c to 3310006 Compare June 12, 2024 16:36

wendigo merged commit 7f7fe5d into trinodb:master Jun 13, 2024
52 checks passed

github-actions bot added this to the 450 milestone Jun 13, 2024

colebow mentioned this pull request Jun 18, 2024

Add Trino 450 release notes #22327

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nessie: Support APIv2 client #22215

Nessie: Support APIv2 client #22215

ajantha-bhat commented May 31, 2024

ajantha-bhat commented May 31, 2024

ajantha-bhat May 31, 2024

wendigo Jun 5, 2024

ajantha-bhat Jun 6, 2024

dimas-b left a comment

ajantha-bhat commented Jun 4, 2024

ajantha-bhat commented Jun 6, 2024

wendigo left a comment

wendigo Jun 6, 2024

ajantha-bhat Jun 12, 2024 •

edited

Loading

wendigo Jun 12, 2024

wendigo Jun 12, 2024

wendigo Jun 12, 2024

wendigo Jun 12, 2024

mosabua Jun 12, 2024

mosabua Jun 12, 2024

JunseoChoJJ commented Jun 13, 2024 •

edited

Loading

ajantha-bhat commented Jun 13, 2024

JunseoChoJJ commented Jun 14, 2024

ajantha-bhat commented Jun 14, 2024

	- Client API version to be used. By default API version is inferred from the `iceberg.nessie-catalog.uri`.
	- Optional version of the Client API version to use. By default it is inferred from the `iceberg.nessie-catalog.uri` value.

	Available values are `V1` or `V2`.
	Valid values are `V1` or `V2`.

Nessie: Support APIv2 client #22215

Nessie: Support APIv2 client #22215

Conversation

ajantha-bhat commented May 31, 2024

Description

Additional context and related issues

Release notes

ajantha-bhat commented May 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimas-b left a comment

Choose a reason for hiding this comment

ajantha-bhat commented Jun 4, 2024

ajantha-bhat commented Jun 6, 2024

wendigo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajantha-bhat Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JunseoChoJJ commented Jun 13, 2024 • edited Loading

ajantha-bhat commented Jun 13, 2024

JunseoChoJJ commented Jun 14, 2024

ajantha-bhat commented Jun 14, 2024

ajantha-bhat Jun 12, 2024 •

edited

Loading

JunseoChoJJ commented Jun 13, 2024 •

edited

Loading