gaborkaszab commented on code in PR #14808:
URL: https://github.com/apache/iceberg/pull/14808#discussion_r2605604107
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -118,7 +118,8 @@
public class TestRESTCatalog extends CatalogTests<RESTCatalog> {
private static final ObjectMapper MAPPER = RESTObjectMapper.mapper();
private static final ResourcePaths RESOURCE_PATHS =
- ResourcePaths.forCatalogProperties(Maps.newHashMap());
+ ResourcePaths.forCatalogProperties(
Review Comment:
I've just gave this another look. What I see is that most of the tests use
the new separator both for the client and the server. There are some tests that
override the response on the config endpoint for different purposes (add
idempotency, enforce table/namespace exists endpoints being used), and these
tests set the old separator in the clients, but as a side-effect of how the
tests are structured. Additionally, these tests don't have multi-level
namespaces so don't exercise any of the new functionality.
My proposal is to configure TestRESTCatalog.RESOURCE_PATHS as in this PR
(this is used for verifications anyway), and since there is no actual test
where the client uses the old separator with multi-level namespaces, I'd add a
new test that explicitly covers this scenario (preferably in this same PR).
WDYT @nastra ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
