ayushtkn commented on code in PR #4966:
URL: https://github.com/apache/polaris/pull/4966#discussion_r3533756593
##########
runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogTest.java:
##########
@@ -1316,15 +1306,10 @@ public void
testUpdateNotificationCreateTableInExternalLocation() {
final String anotherTableLocation =
String.format("s3://my-bucket/path/to/data/another_table_%s/",
tableSuffix);
- metaStoreManager.updateEntityPropertiesIfNotChanged(
- polarisContext,
- List.of(PolarisEntity.toCore(catalogEntity)),
- new CatalogEntity.Builder(CatalogEntity.of(catalogEntity))
- .addProperty(
-
FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "false")
- .addProperty(
-
FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true")
- .build());
+ updateCatalogProperties(
+ Map.of(
+
FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "false",
+
FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(),
"true"));
Review Comment:
> Same: setting ALLOW_EXTERNAL_METADATA_FILE_LOCATION does not see to be
necessary here. The test passed without it in my env. 🤔
There is no `ALLOW_EXTERNAL_METADATA_FILE_LOCATION` here 🤔. Am I reading the
wrong lines?
These two were already there but not calling the available
`updateCatalogProperties` but direclty doing
``metaStoreManager.updateEntityPropertiesIfNotChanged(
polarisContext,``, So, I just picked the exact same configs, just
the method. Let me know If I am catching it wrong
>I wonder about other cases of putting ALLOW_EXTERNAL_METADATA_FILE_LOCATION
into catalog properties. The settings were present before, but because they
were not loaded from catalog properties before this PR, they are probably
useless... WDYT?
All these configs have a default `false` which is the most relevant case. In
general can't say for all, but mostly we too disable for even HiveCatalog in
Iceberg to have any file outside the Table directory, so, the default `false`
is the most idle case, so I believe most of the people won't tweak all these
configs ever unless there is some specific use case.
--
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]
