Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nessie: Support ApiV2 for Nessie client #6712

Merged
merged 5 commits into from
Jun 15, 2023
Merged

Nessie: Support ApiV2 for Nessie client #6712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
314356b
Nessie: Support ApiV2 for Nessie client
ajantha-bhat Jan 31, 2023
6578c25
Address new comments
ajantha-bhat May 20, 2023
505bd44
Address review comments and fix issues with Nessie version bump
ajantha-bhat May 26, 2023
ea141a4
Add prefix for const name
ajantha-bhat May 26, 2023
c5e65d6
handle nit
ajantha-bhat Jun 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.projectnessie.client.NessieClientBuilder;
import org.projectnessie.client.NessieConfigConstants;
import org.projectnessie.client.api.NessieApiV1;
import org.projectnessie.client.api.NessieApiV2;
import org.projectnessie.client.http.HttpClientBuilder;
import org.projectnessie.model.ContentKey;
import org.projectnessie.model.TableReference;
Expand Down Expand Up @@ -88,11 +89,28 @@ public void initialize(String name, Map<String, String> options) {
options.get(removePrefix.apply(NessieConfigConstants.CONF_NESSIE_REF));
String requestedHash =
options.get(removePrefix.apply(NessieConfigConstants.CONF_NESSIE_REF_HASH));
NessieApiV1 api =

NessieClientBuilder<?> nessieClientBuilder =
createNessieClientBuilder(
options.get(NessieConfigConstants.CONF_NESSIE_CLIENT_BUILDER_IMPL))
.fromConfig(x -> options.get(removePrefix.apply(x)))
.build(NessieApiV1.class);
.fromConfig(x -> options.get(removePrefix.apply(x)));
// default version is set to v1.
final String apiVersion =
options.getOrDefault(removePrefix.apply(NessieUtil.CLIENT_API_VERSION), "1");
NessieApiV1 api;
switch (apiVersion) {
case "1":
api = nessieClientBuilder.build(NessieApiV1.class);
break;
case "2":
api = nessieClientBuilder.build(NessieApiV2.class);
break;
default:
throw new IllegalArgumentException(
String.format(
"Unsupported %s: %s. Can only be 1 or 2",
removePrefix.apply(NessieUtil.CLIENT_API_VERSION), apiVersion));
}

initialize(
name,
Expand Down
2 changes: 2 additions & 0 deletions nessie/src/main/java/org/apache/iceberg/nessie/NessieUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public final class NessieUtil {
public static final String NESSIE_CONFIG_PREFIX = "nessie.";
static final String APPLICATION_TYPE = "application-type";

public static final String CLIENT_API_VERSION = "nessie.client-api-version";

private NessieUtil() {}

static TableIdentifier removeCatalogName(TableIdentifier to, String name) {
Expand Down
7 changes: 5 additions & 2 deletions nessie/src/test/java/org/apache/iceberg/nessie/BaseTestIceberg.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
@ExtendWith(DatabaseAdapterExtension.class)
@NessieDbAdapterName(InmemoryDatabaseAdapterFactory.NAME)
@NessieExternalDatabase(InmemoryTestConnectionProviderSource.class)
@NessieApiVersions(versions = NessieApiVersion.V1)
@NessieApiVersions // test all versions
public abstract class BaseTestIceberg {

@NessieDbAdapter static DatabaseAdapter databaseAdapter;
Expand All @@ -88,6 +88,7 @@ public abstract class BaseTestIceberg {

protected NessieCatalog catalog;
protected NessieApiV1 api;
protected String apiVersion;
protected Configuration hadoopConfig;
protected final String branch;
private String initialHashOfDefaultBranch;
Expand Down Expand Up @@ -122,6 +123,7 @@ public void beforeEach(NessieClientFactory clientFactory, @NessieClientUri URI n
throws IOException {
this.uri = nessieUri.toASCIIString();
this.api = clientFactory.make();
this.apiVersion = clientFactory.apiVersion() == NessieApiVersion.V2 ? "2" : "1";

Branch defaultBranch = api.getDefaultBranch();
initialHashOfDefaultBranch = defaultBranch.getHash();
Expand All @@ -145,7 +147,8 @@ NessieCatalog initCatalog(String ref, String hash) {
.put("ref", ref)
.put(CatalogProperties.URI, uri)
.put("auth-type", "NONE")
.put(CatalogProperties.WAREHOUSE_LOCATION, temp.toUri().toString());
.put(CatalogProperties.WAREHOUSE_LOCATION, temp.toUri().toString())
.put("client-api-version", apiVersion);
ajantha-bhat marked this conversation as resolved.
Show resolved Hide resolved
if (null != hash) {
options.put("ref.hash", hash);
}
Expand Down
8 changes: 6 additions & 2 deletions nessie/src/test/java/org/apache/iceberg/nessie/TestCustomNessieClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ public void testNoCustomClient() {
CatalogProperties.WAREHOUSE_LOCATION,
temp.toUri().toString(),
CatalogProperties.URI,
uri));
uri,
"client-api-version",
apiVersion));
}

@Test
Expand All @@ -62,7 +64,9 @@ public void testUnnecessaryDefaultCustomClient() {
CatalogProperties.URI,
uri,
NessieConfigConstants.CONF_NESSIE_CLIENT_BUILDER_IMPL,
HttpClientBuilder.class.getName()));
HttpClientBuilder.class.getName(),
"client-api-version",
apiVersion));
}

@Test
Expand Down
14 changes: 8 additions & 6 deletions nessie/src/test/java/org/apache/iceberg/nessie/TestNessieCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
@ExtendWith(DatabaseAdapterExtension.class)
@NessieDbAdapterName(InmemoryDatabaseAdapterFactory.NAME)
@NessieExternalDatabase(InmemoryTestConnectionProviderSource.class)
@NessieApiVersions(versions = NessieApiVersion.V1)
@NessieApiVersions // test all versions
public class TestNessieCatalog extends CatalogTests<NessieCatalog> {

@NessieDbAdapter static DatabaseAdapter databaseAdapter;
Expand All @@ -67,6 +67,7 @@ public class TestNessieCatalog extends CatalogTests<NessieCatalog> {

private NessieCatalog catalog;
private NessieApiV1 api;
private NessieApiVersion apiVersion;
private Configuration hadoopConfig;
private String initialHashOfDefaultBranch;
private String uri;
Expand All @@ -75,6 +76,7 @@ public class TestNessieCatalog extends CatalogTests<NessieCatalog> {
public void setUp(NessieClientFactory clientFactory, @NessieClientUri URI nessieUri)
throws NessieNotFoundException {
api = clientFactory.make();
apiVersion = clientFactory.apiVersion();
initialHashOfDefaultBranch = api.getDefaultBranch().getHash();
uri = nessieUri.toASCIIString();
hadoopConfig = new Configuration();
Expand Down Expand Up @@ -115,17 +117,17 @@ private void resetData() throws NessieConflictException, NessieNotFoundException
private NessieCatalog initNessieCatalog(String ref) {
NessieCatalog newCatalog = new NessieCatalog();
newCatalog.setConf(hadoopConfig);
newCatalog.initialize(
"nessie",
ImmutableMap<String, String> options =
ImmutableMap.of(
"ref",
ref,
CatalogProperties.URI,
uri,
"auth-type",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that not needed anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default value NONE itself. Hence, I just removed it to make less verbose.

"NONE",
CatalogProperties.WAREHOUSE_LOCATION,
temp.toUri().toString()));
temp.toUri().toString(),
"client-api-version",
apiVersion == NessieApiVersion.V2 ? "2" : "1");
newCatalog.initialize("nessie", options);
return newCatalog;
}

Expand Down
13 changes: 13 additions & 0 deletions nessie/src/test/java/org/apache/iceberg/nessie/TestNessieIcebergClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.apache.iceberg.nessie;

import java.io.IOException;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.assertj.core.api.Assertions;
Expand Down Expand Up @@ -89,4 +90,16 @@ public void testWithReferenceAfterRecreatingBranch()
.isEqualTo(ref);
Assertions.assertThat(client.withReference(branch, null)).isNotEqualTo(client);
}

@Test
public void testInvalidClientApiVersion() throws IOException {
try (NessieCatalog newCatalog = new NessieCatalog()) {
newCatalog.setConf(hadoopConfig);
ImmutableMap.Builder<String, String> options =
ImmutableMap.<String, String>builder().put("client-api-version", "3");
Assertions.assertThatThrownBy(() -> newCatalog.initialize("nessie", options.buildOrThrow()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Unsupported client-api-version: 3. Can only be 1 or 2");
}
}
}