You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "tom-s-powell (via GitHub)" <gi...@apache.org> on 2025/11/17 18:21:26 UTC
[PR] Pass catalog properties to client.credentials-provider [iceberg]
tom-s-powell opened a new pull request, #14608:
URL: https://github.com/apache/iceberg/pull/14608
The goal here is to make catalog-level properties available to https://github.com/apache/iceberg/blob/main/aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java. Given the use of `AuthManager` within this implementation, it would be nice that this could be constructed in a consistent way to the catalog (e.g., `RESTCatalog`) without needing to provide duplicate properties prefixed with `client.credentials-provider`. For example, if I configure `CatalogProperties.URI` it would be nice to not have to configure `client.credentials-provider.uri` also.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: Merge catalog properties with properties prefixed with client.credentials-provider. [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#issuecomment-3715127275
Hey sorry for missing this @tom-s-powell , taking a look
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2581146739
##########
core/src/test/java/org/apache/iceberg/util/TestPropertyUtil.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.util;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Map;
+import org.junit.jupiter.api.Test;
+
+public class TestPropertyUtil {
+
+ @Test
+ void testMergeProperties() {
Review Comment:
```suggestion
void mergeProperties() {
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580845332
##########
core/src/main/java/org/apache/iceberg/util/PropertyUtil.java:
##########
@@ -191,6 +193,21 @@ public static Map<String, String> filterProperties(
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
+ public static Map<String, String> mergeProperties(
Review Comment:
can you please add a test for this to `TestPropertyUtil` (class currently doesn't exist)?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: Merge catalog properties with properties prefixed with client.credentials-provider. [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2581188390
##########
core/src/main/java/org/apache/iceberg/util/PropertyUtil.java:
##########
@@ -27,6 +27,7 @@
import org.apache.iceberg.TableProperties;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
Review Comment:
```suggestion
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580863160
##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsClientProperties.java:
##########
@@ -189,17 +189,22 @@ public void refreshCredentialsEndpointWithOverridingOAuthToken() {
"specific-token");
AwsClientProperties awsClientProperties = new AwsClientProperties(properties);
- Map<String, String> expectedProperties =
- ImmutableMap.<String, String>builder()
- .putAll(properties)
- .put("credentials.uri", "http://localhost:1234/v1/credentials")
- .build();
-
AwsCredentialsProvider provider =
awsClientProperties.credentialsProvider("key", "secret", "token");
assertThat(provider).isInstanceOf(VendedCredentialsProvider.class);
VendedCredentialsProvider vendedCredentialsProvider = (VendedCredentialsProvider) provider;
- assertThat(vendedCredentialsProvider).extracting("properties").isEqualTo(expectedProperties);
+ assertThat(vendedCredentialsProvider)
Review Comment:
sorry for the late reply. I actually didn't realize that the specific-token hasn't been overriding the normal token in tests and it seems that this was introduced by https://github.com/apache/iceberg/commit/e55f23858848bb848a1a16ae1ee045139ffe89fc and missed by me during review. The behavior you're (re-)introducing makes sense
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra closed pull request #14608: Pass catalog properties to client.credentials-provider
URL: https://github.com/apache/iceberg/pull/14608
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2556591683
##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsClientProperties.java:
##########
@@ -189,17 +189,22 @@ public void refreshCredentialsEndpointWithOverridingOAuthToken() {
"specific-token");
AwsClientProperties awsClientProperties = new AwsClientProperties(properties);
- Map<String, String> expectedProperties =
- ImmutableMap.<String, String>builder()
- .putAll(properties)
- .put("credentials.uri", "http://localhost:1234/v1/credentials")
- .build();
-
AwsCredentialsProvider provider =
awsClientProperties.credentialsProvider("key", "secret", "token");
assertThat(provider).isInstanceOf(VendedCredentialsProvider.class);
VendedCredentialsProvider vendedCredentialsProvider = (VendedCredentialsProvider) provider;
- assertThat(vendedCredentialsProvider).extracting("properties").isEqualTo(expectedProperties);
+ assertThat(vendedCredentialsProvider)
Review Comment:
@tom-s-powell what exactly changed now in terms of behavior between the properties we ended up with previously vs now? I think it would be helpful if the exact difference in terms of behavior can be highlighted
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580954084
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -106,15 +106,20 @@ public AwsClientProperties() {
this.refreshCredentialsEndpoint = null;
this.refreshCredentialsEnabled = true;
this.legacyMd5pluginEnabled = false;
- this.allProperties = null;
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
this.clientRegion = properties.get(CLIENT_REGION);
this.clientCredentialsProvider = properties.get(CLIENT_CREDENTIALS_PROVIDER);
+ // Retain all non-prefixed properties and override with prefixed properties
this.clientCredentialsProviderProperties =
- PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX);
+ SerializableMap.copyOf(
+ PropertyUtil.mergeProperties(
+ PropertyUtil.filterProperties(
+ properties,
+ Predicate.not(
+ property -> property.startsWith(CLIENT_CREDENTIAL_PROVIDER_PREFIX))),
+ PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX)));
Review Comment:
These look reasonable to me
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580846947
##########
core/src/main/java/org/apache/iceberg/util/PropertyUtil.java:
##########
@@ -191,6 +193,21 @@ public static Map<String, String> filterProperties(
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
+ public static Map<String, String> mergeProperties(
+ Map<String, String> properties, Map<String, String> overrides) {
+ if (overrides == null || overrides.isEmpty()) {
+ return properties;
+ }
+
+ if (properties == null || properties.isEmpty()) {
+ return overrides;
+ }
+
+ Map<String, String> merged = Maps.newHashMap(properties);
+ merged.putAll(overrides);
+ return ImmutableMap.copyOf(Maps.filterValues(merged, Objects::nonNull));
Review Comment:
I don't think we should be filtering here, as the method indicates that it does a merge only. Also let's just remove the merged map instead of the ImmutableMap
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580936655
##########
core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponse.java:
##########
@@ -109,14 +109,8 @@ public Map<String, String> merge(Map<String, String> clientProperties) {
Preconditions.checkNotNull(
clientProperties,
"Cannot merge client properties with server-provided properties. Invalid client configuration: null");
- Map<String, String> merged = defaults != null ? Maps.newHashMap(defaults) : Maps.newHashMap();
Review Comment:
Fair enough, removed
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -106,15 +106,20 @@ public AwsClientProperties() {
this.refreshCredentialsEndpoint = null;
this.refreshCredentialsEnabled = true;
this.legacyMd5pluginEnabled = false;
- this.allProperties = null;
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
this.clientRegion = properties.get(CLIENT_REGION);
this.clientCredentialsProvider = properties.get(CLIENT_CREDENTIALS_PROVIDER);
+ // Retain all non-prefixed properties and override with prefixed properties
this.clientCredentialsProviderProperties =
- PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX);
+ SerializableMap.copyOf(
Review Comment:
removed
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580935195
##########
core/src/main/java/org/apache/iceberg/util/PropertyUtil.java:
##########
@@ -191,6 +193,21 @@ public static Map<String, String> filterProperties(
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
+ public static Map<String, String> mergeProperties(
Review Comment:
Added a set of test cases
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: Merge catalog properties with properties prefixed with client.credentials-provider. [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#issuecomment-3626373940
Did you have thoughts on this @amogh-jahagirdar?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2537776553
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -106,15 +107,20 @@ public AwsClientProperties() {
this.refreshCredentialsEndpoint = null;
this.refreshCredentialsEnabled = true;
this.legacyMd5pluginEnabled = false;
- this.allProperties = null;
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
this.clientRegion = properties.get(CLIENT_REGION);
this.clientCredentialsProvider = properties.get(CLIENT_CREDENTIALS_PROVIDER);
+ // Retain all non-prefixed properties and override with prefixed properties
this.clientCredentialsProviderProperties =
- PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX);
+ Maps.newHashMap(
Review Comment:
@nastra I agree with your comment. This is where the prefixed properties (e.g., `client.credentials-provider.uri`) would override the catalog properties (e.g., `uri`). You would still get the flexibility to pass properties specific to `VendedCredentialsProvider` that should take priority of other properties.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2537776553
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -106,15 +107,20 @@ public AwsClientProperties() {
this.refreshCredentialsEndpoint = null;
this.refreshCredentialsEnabled = true;
this.legacyMd5pluginEnabled = false;
- this.allProperties = null;
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
this.clientRegion = properties.get(CLIENT_REGION);
this.clientCredentialsProvider = properties.get(CLIENT_CREDENTIALS_PROVIDER);
+ // Retain all non-prefixed properties and override with prefixed properties
this.clientCredentialsProviderProperties =
- PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX);
+ Maps.newHashMap(
Review Comment:
@nastra I agree with your comment. This is where the prefixed properties (e.g., `client.credentials-provider.uri`) would override the catalog properties (e.g., `uri`).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: Merge catalog properties with properties prefixed with client.credentials-provider. [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2665526186
##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsClientProperties.java:
##########
@@ -180,7 +180,7 @@ public void refreshCredentialsEndpointWithOverridingOAuthToken() {
Map<String, String> properties =
ImmutableMap.of(
CatalogProperties.URI,
- "http://localhost:1234/v1",
+ "http://localhost:1234/v1/catalog",
Review Comment:
Just as a note to any other reviewers, this change isn't indicative of a behavior change it really is just consistency in the test setup.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#issuecomment-3546884662
> For example, if I configure CatalogProperties.URI it would be nice to not have to configure client.credentials-provider.uri
The `client.credentials-provider.uri` could be technically a different server from the actual catalog and so we must set both (even if they are pointing to the same server)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580655649
##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsClientProperties.java:
##########
@@ -189,17 +189,22 @@ public void refreshCredentialsEndpointWithOverridingOAuthToken() {
"specific-token");
AwsClientProperties awsClientProperties = new AwsClientProperties(properties);
- Map<String, String> expectedProperties =
- ImmutableMap.<String, String>builder()
- .putAll(properties)
- .put("credentials.uri", "http://localhost:1234/v1/credentials")
- .build();
-
AwsCredentialsProvider provider =
awsClientProperties.credentialsProvider("key", "secret", "token");
assertThat(provider).isInstanceOf(VendedCredentialsProvider.class);
VendedCredentialsProvider vendedCredentialsProvider = (VendedCredentialsProvider) provider;
- assertThat(vendedCredentialsProvider).extracting("properties").isEqualTo(expectedProperties);
+ assertThat(vendedCredentialsProvider)
Review Comment:
@nastra wanted to check back here and see whether my explanation made sense?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2581145630
##########
core/src/main/java/org/apache/iceberg/util/PropertyUtil.java:
##########
@@ -191,6 +192,21 @@ public static Map<String, String> filterProperties(
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
+ public static Map<String, String> mergeProperties(
+ Map<String, String> properties, Map<String, String> overrides) {
+ if (overrides == null || overrides.isEmpty()) {
+ return properties;
+ }
+
+ if (properties == null || properties.isEmpty()) {
+ return overrides;
+ }
+
+ Map<String, String> merged = Maps.newHashMap(properties);
+ merged.putAll(overrides);
+ return ImmutableMap.copyOf(merged);
Review Comment:
```suggestion
return merged;
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580872710
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -106,15 +106,20 @@ public AwsClientProperties() {
this.refreshCredentialsEndpoint = null;
this.refreshCredentialsEnabled = true;
this.legacyMd5pluginEnabled = false;
- this.allProperties = null;
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
this.clientRegion = properties.get(CLIENT_REGION);
this.clientCredentialsProvider = properties.get(CLIENT_CREDENTIALS_PROVIDER);
+ // Retain all non-prefixed properties and override with prefixed properties
this.clientCredentialsProviderProperties =
- PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX);
+ SerializableMap.copyOf(
+ PropertyUtil.mergeProperties(
+ PropertyUtil.filterProperties(
+ properties,
+ Predicate.not(
+ property -> property.startsWith(CLIENT_CREDENTIAL_PROVIDER_PREFIX))),
+ PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX)));
Review Comment:
can you please also check GCPProperties and AzureProperties to make sure we do the correct thing there as well in terms of properties?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580856099
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -106,15 +106,20 @@ public AwsClientProperties() {
this.refreshCredentialsEndpoint = null;
this.refreshCredentialsEnabled = true;
this.legacyMd5pluginEnabled = false;
- this.allProperties = null;
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
this.clientRegion = properties.get(CLIENT_REGION);
this.clientCredentialsProvider = properties.get(CLIENT_CREDENTIALS_PROVIDER);
+ // Retain all non-prefixed properties and override with prefixed properties
this.clientCredentialsProviderProperties =
- PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX);
+ SerializableMap.copyOf(
Review Comment:
no need to use `SerializableMap.copyOf` here
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: Merge catalog properties with properties prefixed with client.credentials-provider. [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2581195457
##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsClientProperties.java:
##########
@@ -189,17 +189,22 @@ public void refreshCredentialsEndpointWithOverridingOAuthToken() {
"specific-token");
AwsClientProperties awsClientProperties = new AwsClientProperties(properties);
- Map<String, String> expectedProperties =
- ImmutableMap.<String, String>builder()
- .putAll(properties)
- .put("credentials.uri", "http://localhost:1234/v1/credentials")
- .build();
-
AwsCredentialsProvider provider =
awsClientProperties.credentialsProvider("key", "secret", "token");
assertThat(provider).isInstanceOf(VendedCredentialsProvider.class);
VendedCredentialsProvider vendedCredentialsProvider = (VendedCredentialsProvider) provider;
- assertThat(vendedCredentialsProvider).extracting("properties").isEqualTo(expectedProperties);
+ assertThat(vendedCredentialsProvider)
Review Comment:
@tom-s-powell can you update the PR description to mention what's currently broken and what the PR addresses with a link to the commit that changed the behavior?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2542591230
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -106,15 +107,20 @@ public AwsClientProperties() {
this.refreshCredentialsEndpoint = null;
this.refreshCredentialsEnabled = true;
this.legacyMd5pluginEnabled = false;
- this.allProperties = null;
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
this.clientRegion = properties.get(CLIENT_REGION);
this.clientCredentialsProvider = properties.get(CLIENT_CREDENTIALS_PROVIDER);
+ // Retain all non-prefixed properties and override with prefixed properties
this.clientCredentialsProviderProperties =
- PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX);
+ Maps.newHashMap(
Review Comment:
I don't think I'm fully following. Why would `client.credentials-provider.uri` override `uri`? Those are two separate properties that control two separate things
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2535082563
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -211,7 +220,6 @@ public <T extends S3CrtAsyncClientBuilder> void applyClientCredentialConfigurati
public AwsCredentialsProvider credentialsProvider(
String accessKeyId, String secretAccessKey, String sessionToken) {
if (refreshCredentialsEnabled && !Strings.isNullOrEmpty(refreshCredentialsEndpoint)) {
- clientCredentialsProviderProperties.putAll(allProperties);
Review Comment:
This would now be covered by the code above where on `initialize` we propagate `allProperties` to `clientCredentialsProviderProperties`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: Merge catalog properties with properties prefixed with client.credentials-provider. [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#issuecomment-3602065248
@amogh-jahagirdar could you review this one as well please? See https://github.com/apache/iceberg/pull/14608#discussion_r2580863160 for some additional details about what's currently broken
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: Merge catalog properties with properties prefixed with client.credentials-provider. [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #14608:
URL: https://github.com/apache/iceberg/pull/14608
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: Merge catalog properties with properties prefixed with client.credentials-provider. [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2665488996
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -106,15 +105,18 @@ public AwsClientProperties() {
this.refreshCredentialsEndpoint = null;
this.refreshCredentialsEnabled = true;
this.legacyMd5pluginEnabled = false;
- this.allProperties = null;
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
this.clientRegion = properties.get(CLIENT_REGION);
this.clientCredentialsProvider = properties.get(CLIENT_CREDENTIALS_PROVIDER);
+ // Retain all non-prefixed properties and override with prefixed properties
this.clientCredentialsProviderProperties =
- PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX);
+ PropertyUtil.mergeProperties(
+ PropertyUtil.filterProperties(
+ properties,
+ Predicate.not(property -> property.startsWith(CLIENT_CREDENTIAL_PROVIDER_PREFIX))),
+ PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX));
Review Comment:
Yeah this looks right. The client credentials properties needs to override the base properties if defined. So this will have a merged set of non-prefixed properties AND any overriden or added prefixed properties.
##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsClientProperties.java:
##########
@@ -189,17 +189,22 @@ public void refreshCredentialsEndpointWithOverridingOAuthToken() {
"specific-token");
AwsClientProperties awsClientProperties = new AwsClientProperties(properties);
- Map<String, String> expectedProperties =
- ImmutableMap.<String, String>builder()
- .putAll(properties)
- .put("credentials.uri", "http://localhost:1234/v1/credentials")
- .build();
-
AwsCredentialsProvider provider =
awsClientProperties.credentialsProvider("key", "secret", "token");
assertThat(provider).isInstanceOf(VendedCredentialsProvider.class);
VendedCredentialsProvider vendedCredentialsProvider = (VendedCredentialsProvider) provider;
- assertThat(vendedCredentialsProvider).extracting("properties").isEqualTo(expectedProperties);
+ assertThat(vendedCredentialsProvider)
+ .extracting("properties")
+ .isEqualTo(
+ ImmutableMap.of(
+ AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT,
+ "http://localhost:1234/v1/credentials",
+ "credentials.uri",
+ "http://localhost:1234/v1/credentials",
+ CatalogProperties.URI,
+ "http://localhost:1234/v1/catalog",
+ OAuth2Properties.TOKEN,
+ "specific-token"));
Review Comment:
Looks right, this is overriding the specific token instead of having a separate base one and a client prefixed one (and it doesn't look like VendedCredentialsProvider does any additional work to filter prefix scoped properties, it looks like it just expects those to be given to it)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2542821876
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -106,15 +107,20 @@ public AwsClientProperties() {
this.refreshCredentialsEndpoint = null;
this.refreshCredentialsEnabled = true;
this.legacyMd5pluginEnabled = false;
- this.allProperties = null;
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
this.clientRegion = properties.get(CLIENT_REGION);
this.clientCredentialsProvider = properties.get(CLIENT_CREDENTIALS_PROVIDER);
+ // Retain all non-prefixed properties and override with prefixed properties
this.clientCredentialsProviderProperties =
- PropertyUtil.propertiesWithPrefix(properties, CLIENT_CREDENTIAL_PROVIDER_PREFIX);
+ Maps.newHashMap(
Review Comment:
Hmm. There is `credentials.uri` which configures the [credentialEndpoint](https://github.com/apache/iceberg/blob/main/aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java#L67) and `uri` which configures the [catalogEndpoint](https://github.com/apache/iceberg/blob/main/aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java#L66).
`client.credentials-provider.uri` is still effectively the `catalogEndpoint` though, since the code takes properties with the prefix `client.credentials-provider.` and propagates those to `VendedCredentialsProvider`. So, if I were to configure `uri` for my `RESTCatalog` I would need to provide `client.credentials-provider.uri` to configure that same URI for `VendedCredentialsProvider`.
Note that this is _not_ the case when the `VendedCredentialsProvider` is constructed via https://github.com/apache/iceberg/blob/main/aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java#L211, because this code path does pass the original properties through that were configured when initializing the `RESTCatalog`. On that code path I wouldn't also need to have configured `client.credentials-provider.uri` when I have set `uri` in the properties (which naturally you would when initializing a `RESTCatalog`).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2556691337
##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsClientProperties.java:
##########
@@ -180,7 +180,7 @@ public void refreshCredentialsEndpointWithOverridingOAuthToken() {
Map<String, String> properties =
ImmutableMap.of(
CatalogProperties.URI,
- "http://localhost:1234/v1",
+ "http://localhost:1234/v1/catalog",
Review Comment:
updating this just for consistency with `refreshCredentialsEndpointWithOAuthToken`
##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsClientProperties.java:
##########
@@ -189,17 +189,22 @@ public void refreshCredentialsEndpointWithOverridingOAuthToken() {
"specific-token");
AwsClientProperties awsClientProperties = new AwsClientProperties(properties);
- Map<String, String> expectedProperties =
- ImmutableMap.<String, String>builder()
- .putAll(properties)
- .put("credentials.uri", "http://localhost:1234/v1/credentials")
- .build();
-
AwsCredentialsProvider provider =
awsClientProperties.credentialsProvider("key", "secret", "token");
assertThat(provider).isInstanceOf(VendedCredentialsProvider.class);
VendedCredentialsProvider vendedCredentialsProvider = (VendedCredentialsProvider) provider;
- assertThat(vendedCredentialsProvider).extracting("properties").isEqualTo(expectedProperties);
+ assertThat(vendedCredentialsProvider)
Review Comment:
Previous:
```
{
"client.credentials-provider.token": "specific-token",
"client.refresh-credentials-endpoint": "http://localhost:1234/v1/credentials",
"credentials.uri": "http://localhost:1234/v1/credentials",
"token": "oauth-token",
"uri": "http://localhost:1234/v1/catalog"
}
```
New:
```
{
"client.refresh-credentials-endpoint": "http://localhost:1234/v1/credentials",
"credentials.uri": "http://localhost:1234/v1/credentials",
"token": "specific-token",
"uri": "http://localhost:1234/v1/catalog"
}
```
Here you can see that `VendedCredentialsProvider` is now receiving a single `token` property rather than `token` and `client.credentials-provider.token`. However, the value of `token` it receives is actually the value of `client.credentials-provider.token` which has overridden the original `token` property.
My impression from reading the implementation of `VendedCredentialsProvider` is that this is actually more desirable behaviour, in that (a) it never references the prefixed properties and (b) `PropertyUtil.propertiesWithPrefix` filters properties with a prefix then removes the prefix (i.e., you would not expect to receive the prefixed property).
I'm kind of confused what this test was previously testing as it didn't seem to be reflecting any property overrides.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] AWS: Merge catalog properties with properties prefixed with client.credentials-provider. [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#issuecomment-3714650276
@amogh-jahagirdar - wanted to check back in here and see if you had thoughts?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Pass catalog properties to client.credentials-provider [iceberg]
Posted by "tom-s-powell (via GitHub)" <gi...@apache.org>.
tom-s-powell commented on code in PR #14608:
URL: https://github.com/apache/iceberg/pull/14608#discussion_r2580942345
##########
core/src/main/java/org/apache/iceberg/util/PropertyUtil.java:
##########
@@ -191,6 +193,21 @@ public static Map<String, String> filterProperties(
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
+ public static Map<String, String> mergeProperties(
+ Map<String, String> properties, Map<String, String> overrides) {
+ if (overrides == null || overrides.isEmpty()) {
+ return properties;
+ }
+
+ if (properties == null || properties.isEmpty()) {
+ return overrides;
+ }
+
+ Map<String, String> merged = Maps.newHashMap(properties);
+ merged.putAll(overrides);
+ return ImmutableMap.copyOf(Maps.filterValues(merged, Objects::nonNull));
Review Comment:
Fair enough. I had done this to match the behaviour of `ConfigResponse` but given I've removed that I've removed this. One thing to be mindful of is `ImmutableMap` will _not_ accept nulls, and I am still using `ImmutableMap.copyOf`. I guess it is unexpected to have nulls in the place where we're currently using this utility so likely fine.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org