poojanilangekar commented on code in PR #1925:
URL: https://github.com/apache/polaris/pull/1925#discussion_r2183707842
##########
runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAdminServiceAuthzTest.java:
##########
@@ -151,7 +152,8 @@ public void testCreateCatalogSufficientPrivileges() {
@Test
public void testCreateCatalogInsufficientPrivileges() {
- final CatalogEntity newCatalog = new
CatalogEntity.Builder().setName("new_catalog").build();
+ final CatalogEntity newCatalog =
+ new
CatalogEntity.Builder().setName("new_catalog").setCatalogType("INTERNAL").build();
Review Comment:
Polaris assumes any catalog that is not defined as "INTERNAL" is by default
"EXTERNAL", it ends up activating the code-path in PolarisAdminService.java:733
Granted that since we allow legacy `EXTERNAL` catalogs without
ConnectionConfigInfos, the test would still pass without the change. But I
think that wasn't the intention of the test.
For this test and other tests in `catalog/`, I think it's ideal to not
activate that code path in the first place. If you want me to I can revert
this and other changes. Please let me know what you think.
--
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]
