You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "adutra (via GitHub)" <gi...@apache.org> on 2026/05/20 12:30:36 UTC
[PR] Replace deprecated encodeNamespace and decodeNamespace methods (#3713) [polaris]
adutra opened a new pull request, #4508:
URL: https://github.com/apache/polaris/pull/4508
This is a cherry-pick of #3713.
Although the logic has been reviewed and confirmed, the patch didn't apply cleanly; therefore, a brief follow-up review is recommended.
## Checklist
- [ ] ๐ก๏ธ Don't disclose security issues! (contact security@apache.org)
- [ ] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes #
- [ ] ๐งช Added/updated tests with good coverage, or manually tested (and explained how)
- [ ] ๐ก Added comments for complex logic
- [ ] ๐งพ Updated `CHANGELOG.md` (if needed)
- [ ] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3276415754
##########
polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java:
##########
@@ -43,27 +49,46 @@ public class PolarisResourcePaths {
public static final String V1_APPLICABLE_POLICIES = "/polaris/v1/{prefix}/applicable-policies";
private final String prefix;
+ private final String namespaceSeparatorEncoded;
- public PolarisResourcePaths(String prefix) {
+ public PolarisResourcePaths(String prefix, String namespaceSeparatorEncoded) {
this.prefix = prefix;
+ this.namespaceSeparatorEncoded = namespaceSeparatorEncoded;
}
public static PolarisResourcePaths forCatalogProperties(Map<String, String> properties) {
- return new PolarisResourcePaths(properties.get(PREFIX));
+ String namespaceSeparatorEncoded =
+ properties.getOrDefault(
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT);
+ if (!namespaceSeparatorEncoded.equals(RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT)) {
+ LOGGER.warn("Using non-default namespace separator '{}'", namespaceSeparatorEncoded);
Review Comment:
ok :+1:
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3277383653
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1436,10 +1438,16 @@ public ConfigResponse getConfig() {
Map<String, String> properties =
PolarisEntity.of(resolvedReferenceCatalog.getEntity()).getPropertiesAsMap();
- String prefix = prefixParser().catalogNameToPrefix(catalogName());
return ConfigResponse.builder()
.withDefaults(properties) // catalog properties are defaults
- .withOverrides(ImmutableMap.of("prefix", prefix))
+ .withOverrides(
+ ImmutableMap.of(
+ "prefix",
+ prefixParser().catalogNameToPrefix(catalogName()),
+ // Polaris does not handle custom namespace separators;
+ // always communicate the default namespace separator to clients.
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ NamespaceUtils.DEFAULT_NAMESPACE_SEPARATOR_ENCODED))
Review Comment:
Yes, exactly: the IRC spec (implicitly) mandates that the namespace separator be communicated to clients in encoded form, not bare.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3274507193
##########
polaris-core/src/main/java/org/apache/polaris/core/rest/NamespaceUtils.java:
##########
@@ -35,6 +35,19 @@ public final class NamespaceUtils {
*/
public static final String DEFAULT_NAMESPACE_SEPARATOR = "\u001f";
+ /**
+ * The default namespace separator, percent-encoded to "%1F".
+ *
+ * <p>This is the same encoded separator declared in Iceberg's {@link
+ * org.apache.iceberg.rest.RESTUtil} class, but it's package-private there and cannot be
+ * referenced.
+ *
+ * <p>This value is also exposed in {@link
+ * org.apache.iceberg.rest.RESTCatalogProperties#NAMESPACE_SEPARATOR_DEFAULT} โ but here we avoid
+ * referencing that field directly since we don't own it.
+ */
+ public static final String DEFAULT_NAMESPACE_SEPARATOR_ENCODED = "%1F";
Review Comment:
That's the _opposite_ of what we should do ๐
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3276260308
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1436,10 +1438,16 @@ public ConfigResponse getConfig() {
Map<String, String> properties =
PolarisEntity.of(resolvedReferenceCatalog.getEntity()).getPropertiesAsMap();
- String prefix = prefixParser().catalogNameToPrefix(catalogName());
return ConfigResponse.builder()
.withDefaults(properties) // catalog properties are defaults
- .withOverrides(ImmutableMap.of("prefix", prefix))
+ .withOverrides(
+ ImmutableMap.of(
+ "prefix",
+ prefixParser().catalogNameToPrefix(catalogName()),
+ // Polaris does not handle custom namespace separators;
+ // always communicate the default namespace separator to clients.
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ NamespaceUtils.DEFAULT_NAMESPACE_SEPARATOR_ENCODED))
Review Comment:
No, clients cannot choose the separator, if no separator is specified, they pick the default one.
I thought on the contrary that it would be cleaner to explicitly define which separator to use โ even if it's the default one, don't you agree? Especially if later on we decide to allow Polaris to use custom separators.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3274519799
##########
polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java:
##########
@@ -43,27 +49,46 @@ public class PolarisResourcePaths {
public static final String V1_APPLICABLE_POLICIES = "/polaris/v1/{prefix}/applicable-policies";
private final String prefix;
+ private final String namespaceSeparatorEncoded;
- public PolarisResourcePaths(String prefix) {
+ public PolarisResourcePaths(String prefix, String namespaceSeparatorEncoded) {
this.prefix = prefix;
+ this.namespaceSeparatorEncoded = namespaceSeparatorEncoded;
}
public static PolarisResourcePaths forCatalogProperties(Map<String, String> properties) {
- return new PolarisResourcePaths(properties.get(PREFIX));
+ String namespaceSeparatorEncoded =
+ properties.getOrDefault(
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT);
+ if (!namespaceSeparatorEncoded.equals(RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT)) {
+ LOGGER.warn("Using non-default namespace separator '{}'", namespaceSeparatorEncoded);
+ }
+ return new PolarisResourcePaths(properties.get(PREFIX), namespaceSeparatorEncoded);
Review Comment:
Indeed, but this code path is used by the Spark Plugin client as well. If it's talking to a non-vanilla version of Polaris that uses a different separator (or a future Polaris version that _does_ allow custom separators), forcing the separator here would break the communication between the client and that specific Polaris 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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3276456486
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1436,10 +1438,16 @@ public ConfigResponse getConfig() {
Map<String, String> properties =
PolarisEntity.of(resolvedReferenceCatalog.getEntity()).getPropertiesAsMap();
- String prefix = prefixParser().catalogNameToPrefix(catalogName());
return ConfigResponse.builder()
.withDefaults(properties) // catalog properties are defaults
- .withOverrides(ImmutableMap.of("prefix", prefix))
+ .withOverrides(
+ ImmutableMap.of(
+ "prefix",
+ prefixParser().catalogNameToPrefix(catalogName()),
+ // Polaris does not handle custom namespace separators;
+ // always communicate the default namespace separator to clients.
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ NamespaceUtils.DEFAULT_NAMESPACE_SEPARATOR_ENCODED))
Review Comment:
However, this happens to be described in the IRC spec, so Polaris' behaviour is correct: https://github.com/apache/iceberg/blob/apache-iceberg-1.11.0/open-api/rest-catalog-open-api.yaml#L278
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3276425838
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1436,10 +1438,16 @@ public ConfigResponse getConfig() {
Map<String, String> properties =
PolarisEntity.of(resolvedReferenceCatalog.getEntity()).getPropertiesAsMap();
- String prefix = prefixParser().catalogNameToPrefix(catalogName());
return ConfigResponse.builder()
.withDefaults(properties) // catalog properties are defaults
- .withOverrides(ImmutableMap.of("prefix", prefix))
+ .withOverrides(
+ ImmutableMap.of(
+ "prefix",
+ prefixParser().catalogNameToPrefix(catalogName()),
+ // Polaris does not handle custom namespace separators;
+ // always communicate the default namespace separator to clients.
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ NamespaceUtils.DEFAULT_NAMESPACE_SEPARATOR_ENCODED))
Review Comment:
I'd agree in general, but what irks me is the fact the that config value is an URL _encoded_ separator. The client has to know exactly how to apply it to avoid double encoding :thinking:
With defaults, the onus is completely on the client to produce correct URLs.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3276433049
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1436,10 +1438,16 @@ public ConfigResponse getConfig() {
Map<String, String> properties =
PolarisEntity.of(resolvedReferenceCatalog.getEntity()).getPropertiesAsMap();
- String prefix = prefixParser().catalogNameToPrefix(catalogName());
return ConfigResponse.builder()
.withDefaults(properties) // catalog properties are defaults
- .withOverrides(ImmutableMap.of("prefix", prefix))
+ .withOverrides(
+ ImmutableMap.of(
+ "prefix",
+ prefixParser().catalogNameToPrefix(catalogName()),
+ // Polaris does not handle custom namespace separators;
+ // always communicate the default namespace separator to clients.
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ NamespaceUtils.DEFAULT_NAMESPACE_SEPARATOR_ENCODED))
Review Comment:
I'm sure current Iceberg java code will work fine... but I'm not so sure about other clients :thinking:
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3276425838
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1436,10 +1438,16 @@ public ConfigResponse getConfig() {
Map<String, String> properties =
PolarisEntity.of(resolvedReferenceCatalog.getEntity()).getPropertiesAsMap();
- String prefix = prefixParser().catalogNameToPrefix(catalogName());
return ConfigResponse.builder()
.withDefaults(properties) // catalog properties are defaults
- .withOverrides(ImmutableMap.of("prefix", prefix))
+ .withOverrides(
+ ImmutableMap.of(
+ "prefix",
+ prefixParser().catalogNameToPrefix(catalogName()),
+ // Polaris does not handle custom namespace separators;
+ // always communicate the default namespace separator to clients.
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ NamespaceUtils.DEFAULT_NAMESPACE_SEPARATOR_ENCODED))
Review Comment:
I'd agree in general, but what irks me is the fact the that config value is a URL _encoded_ separator. The client has to know exactly how to apply it to avoid double encoding :thinking:
With defaults, the onus is completely on the client to produce correct URLs.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3276209693
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1436,10 +1438,16 @@ public ConfigResponse getConfig() {
Map<String, String> properties =
PolarisEntity.of(resolvedReferenceCatalog.getEntity()).getPropertiesAsMap();
- String prefix = prefixParser().catalogNameToPrefix(catalogName());
return ConfigResponse.builder()
.withDefaults(properties) // catalog properties are defaults
- .withOverrides(ImmutableMap.of("prefix", prefix))
+ .withOverrides(
+ ImmutableMap.of(
+ "prefix",
+ prefixParser().catalogNameToPrefix(catalogName()),
+ // Polaris does not handle custom namespace separators;
+ // always communicate the default namespace separator to clients.
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ NamespaceUtils.DEFAULT_NAMESPACE_SEPARATOR_ENCODED))
Review Comment:
Can clients choose their own NS separator? If we do not set `RESTCatalogProperties.NAMESPACE_SEPARATOR`, isn't the client required to use the default one?
If that works, I think it's cleaner to stick to defaults rather than emitting an explicit config entry whose value is equal to the default.
##########
polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java:
##########
@@ -43,27 +49,46 @@ public class PolarisResourcePaths {
public static final String V1_APPLICABLE_POLICIES = "/polaris/v1/{prefix}/applicable-policies";
private final String prefix;
+ private final String namespaceSeparatorEncoded;
- public PolarisResourcePaths(String prefix) {
+ public PolarisResourcePaths(String prefix, String namespaceSeparatorEncoded) {
this.prefix = prefix;
+ this.namespaceSeparatorEncoded = namespaceSeparatorEncoded;
}
public static PolarisResourcePaths forCatalogProperties(Map<String, String> properties) {
- return new PolarisResourcePaths(properties.get(PREFIX));
+ String namespaceSeparatorEncoded =
+ properties.getOrDefault(
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT);
+ if (!namespaceSeparatorEncoded.equals(RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT)) {
+ LOGGER.warn("Using non-default namespace separator '{}'", namespaceSeparatorEncoded);
Review Comment:
Why WARN? Is it inherently risky?
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3276262556
##########
polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java:
##########
@@ -43,27 +49,46 @@ public class PolarisResourcePaths {
public static final String V1_APPLICABLE_POLICIES = "/polaris/v1/{prefix}/applicable-policies";
private final String prefix;
+ private final String namespaceSeparatorEncoded;
- public PolarisResourcePaths(String prefix) {
+ public PolarisResourcePaths(String prefix, String namespaceSeparatorEncoded) {
this.prefix = prefix;
+ this.namespaceSeparatorEncoded = namespaceSeparatorEncoded;
}
public static PolarisResourcePaths forCatalogProperties(Map<String, String> properties) {
- return new PolarisResourcePaths(properties.get(PREFIX));
+ String namespaceSeparatorEncoded =
+ properties.getOrDefault(
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT);
+ if (!namespaceSeparatorEncoded.equals(RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT)) {
+ LOGGER.warn("Using non-default namespace separator '{}'", namespaceSeparatorEncoded);
Review Comment:
In my opinion, yes, because this is largely uncharted territory.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (#3713) [polaris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4508:
URL: https://github.com/apache/polaris/pull/4508#discussion_r3273940908
##########
polaris-core/src/main/java/org/apache/polaris/core/rest/NamespaceUtils.java:
##########
@@ -35,6 +35,19 @@ public final class NamespaceUtils {
*/
public static final String DEFAULT_NAMESPACE_SEPARATOR = "\u001f";
+ /**
+ * The default namespace separator, percent-encoded to "%1F".
+ *
+ * <p>This is the same encoded separator declared in Iceberg's {@link
+ * org.apache.iceberg.rest.RESTUtil} class, but it's package-private there and cannot be
+ * referenced.
+ *
+ * <p>This value is also exposed in {@link
+ * org.apache.iceberg.rest.RESTCatalogProperties#NAMESPACE_SEPARATOR_DEFAULT} โ but here we avoid
+ * referencing that field directly since we don't own it.
+ */
+ public static final String DEFAULT_NAMESPACE_SEPARATOR_ENCODED = "%1F";
Review Comment:
`DEFAULT_NAMESPACE_SEPARATOR_ENCODED` is hard-coded to "%1F" even though Iceberg exposes the default via `RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT`. To avoid Polaris drifting from Iceberg if the default ever changes (or differs by version), consider deriving this value from the Iceberg constant instead of duplicating it.
##########
polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java:
##########
@@ -43,27 +49,46 @@ public class PolarisResourcePaths {
public static final String V1_APPLICABLE_POLICIES = "/polaris/v1/{prefix}/applicable-policies";
private final String prefix;
+ private final String namespaceSeparatorEncoded;
- public PolarisResourcePaths(String prefix) {
+ public PolarisResourcePaths(String prefix, String namespaceSeparatorEncoded) {
this.prefix = prefix;
+ this.namespaceSeparatorEncoded = namespaceSeparatorEncoded;
}
public static PolarisResourcePaths forCatalogProperties(Map<String, String> properties) {
- return new PolarisResourcePaths(properties.get(PREFIX));
+ String namespaceSeparatorEncoded =
+ properties.getOrDefault(
+ RESTCatalogProperties.NAMESPACE_SEPARATOR,
+ RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT);
+ if (!namespaceSeparatorEncoded.equals(RESTCatalogProperties.NAMESPACE_SEPARATOR_DEFAULT)) {
+ LOGGER.warn("Using non-default namespace separator '{}'", namespaceSeparatorEncoded);
+ }
+ return new PolarisResourcePaths(properties.get(PREFIX), namespaceSeparatorEncoded);
Review Comment:
`forCatalogProperties` currently accepts and uses a non-default `RESTCatalogProperties.NAMESPACE_SEPARATOR` value (only logging a warning). Since Polaris endpoints are parsed using `NamespaceUtils.DEFAULT_NAMESPACE_SEPARATOR` and the service config advertises the default separator, generating client paths with a different separator will produce URLs the server won't understand. Consider forcing the default separator here (or failing fast) instead of honoring a non-default value.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace deprecated encodeNamespace and decodeNamespace methods (cherry-pick of #3713) [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra merged PR #4508:
URL: https://github.com/apache/polaris/pull/4508
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org