You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "plusplusjiajia (via GitHub)" <gi...@apache.org> on 2025/09/10 07:32:00 UTC
[PR] [core] RESTCatalog: add DLF OSS endpoint support and improve configuration merge [paimon]
plusplusjiajia opened a new pull request, #6232:
URL: https://github.com/apache/paimon/pull/6232
### Purpose
When using Paimon with DLF (Data Lake Formation) server, the server passes default OSS endpoint values (e.g., fs.oss.endpoint=oss-cn-hangzhou-internal.aliyuncs.com) through tokens. However, these default internal endpoints are not accessible in all client environments, causing connectivity issues. The client-provided options should take precedence over server defaults, but the previous merge logic didn't handle this correctly. This PR introduced DLF_OSS_ENDPOINT configuration option in RESTCatalogOptions to allow clients to specify their preferred OSS endpoint.
### Tests
org.apache.paimon.utils.RESTUtilTest
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] RESTCatalog: add DLF OSS endpoint support and improve configuration merge [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #6232:
URL: https://github.com/apache/paimon/pull/6232#discussion_r2335975793
##########
paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java:
##########
@@ -72,19 +74,30 @@ public static Map<String, String> merge(
if (updates == null) {
updates = Maps.newHashMap();
}
- ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
- for (Map.Entry<String, String> entry : targets.entrySet()) {
- if (!updates.containsKey(entry.getKey()) && entry.getValue() != null) {
- builder.put(entry.getKey(), entry.getValue());
+
+ Map<String, String> result = Maps.newLinkedHashMap();
+
+ // First, add all non-null entries from updates
+ for (Map.Entry<String, String> entry : updates.entrySet()) {
+ if (entry.getValue() != null) {
+ result.put(entry.getKey(), entry.getValue());
}
}
- for (Map.Entry<String, String> entry : updates.entrySet()) {
+
+ // Then, add entries from targets, which take precedence (override updates)
+ for (Map.Entry<String, String> entry : targets.entrySet()) {
if (entry.getValue() != null) {
- builder.put(entry.getKey(), entry.getValue());
+ result.put(entry.getKey(), entry.getValue());
}
}
- return builder.build();
+ // Handle special case: dlf.oss-endpoint should override fs.oss.endpoint
+ String dlfOssEndpoint = result.get(DLF_OSS_ENDPOINT.key());
Review Comment:
Can you just modify `RESTTokenFileIO`? `RESTUtil.merge` should be a very simple method, just `updates` merge into `targets`.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] RESTCatalog: add DLF OSS endpoint support and improve configuration merge [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #6232:
URL: https://github.com/apache/paimon/pull/6232#discussion_r2335975793
##########
paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java:
##########
@@ -72,19 +74,30 @@ public static Map<String, String> merge(
if (updates == null) {
updates = Maps.newHashMap();
}
- ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
- for (Map.Entry<String, String> entry : targets.entrySet()) {
- if (!updates.containsKey(entry.getKey()) && entry.getValue() != null) {
- builder.put(entry.getKey(), entry.getValue());
+
+ Map<String, String> result = Maps.newLinkedHashMap();
+
+ // First, add all non-null entries from updates
+ for (Map.Entry<String, String> entry : updates.entrySet()) {
+ if (entry.getValue() != null) {
+ result.put(entry.getKey(), entry.getValue());
}
}
- for (Map.Entry<String, String> entry : updates.entrySet()) {
+
+ // Then, add entries from targets, which take precedence (override updates)
+ for (Map.Entry<String, String> entry : targets.entrySet()) {
if (entry.getValue() != null) {
- builder.put(entry.getKey(), entry.getValue());
+ result.put(entry.getKey(), entry.getValue());
}
}
- return builder.build();
+ // Handle special case: dlf.oss-endpoint should override fs.oss.endpoint
+ String dlfOssEndpoint = result.get(DLF_OSS_ENDPOINT.key());
Review Comment:
Just modify `RESTTokenFileIO`?
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] RESTCatalog: add DLF OSS endpoint support and improve configuration merge [paimon]
Posted by "plusplusjiajia (via GitHub)" <gi...@apache.org>.
plusplusjiajia commented on code in PR #6232:
URL: https://github.com/apache/paimon/pull/6232#discussion_r2336197519
##########
paimon-api/src/main/java/org/apache/paimon/rest/RESTUtil.java:
##########
@@ -72,19 +74,30 @@ public static Map<String, String> merge(
if (updates == null) {
updates = Maps.newHashMap();
}
- ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
- for (Map.Entry<String, String> entry : targets.entrySet()) {
- if (!updates.containsKey(entry.getKey()) && entry.getValue() != null) {
- builder.put(entry.getKey(), entry.getValue());
+
+ Map<String, String> result = Maps.newLinkedHashMap();
+
+ // First, add all non-null entries from updates
+ for (Map.Entry<String, String> entry : updates.entrySet()) {
+ if (entry.getValue() != null) {
+ result.put(entry.getKey(), entry.getValue());
}
}
- for (Map.Entry<String, String> entry : updates.entrySet()) {
+
+ // Then, add entries from targets, which take precedence (override updates)
+ for (Map.Entry<String, String> entry : targets.entrySet()) {
if (entry.getValue() != null) {
- builder.put(entry.getKey(), entry.getValue());
+ result.put(entry.getKey(), entry.getValue());
}
}
- return builder.build();
+ // Handle special case: dlf.oss-endpoint should override fs.oss.endpoint
+ String dlfOssEndpoint = result.get(DLF_OSS_ENDPOINT.key());
Review Comment:
> Can you just modify `RESTTokenFileIO`? `RESTUtil.merge` should be a very simple method, just `updates` merge into `targets`.
Sure, updated.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] RESTCatalog: add DLF OSS endpoint support and improve configuration merge [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #6232:
URL: https://github.com/apache/paimon/pull/6232
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org