adutra commented on code in PR #2405:
URL: https://github.com/apache/polaris/pull/2405#discussion_r2307669508


##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewAdlsIntegrationTestBase.java:
##########
@@ -18,29 +18,31 @@
  */
 package org.apache.polaris.service.it.test;
 
-import com.google.common.base.Strings;
+import java.io.File;
 import java.util.List;
-import java.util.stream.Stream;
 import org.apache.polaris.core.admin.model.AzureStorageConfigInfo;
 import org.apache.polaris.core.admin.model.StorageConfigInfo;
+import org.apache.polaris.core.storage.StorageUtil;
 
 /** Runs PolarisRestCatalogViewIntegrationTest on Azure. */
-public class PolarisRestCatalogViewAzureIntegrationTest
+public abstract class PolarisRestCatalogViewAdlsIntegrationTestBase
     extends PolarisRestCatalogViewIntegrationBase {
-  public static final String TENANT_ID = 
System.getenv("INTEGRATION_TEST_AZURE_TENANT_ID");
-  public static final String BASE_LOCATION = 
System.getenv("INTEGRATION_TEST_AZURE_PATH");
+  public static final String TENANT_ID = 
System.getenv("INTEGRATION_TEST_ADLS_TENANT_ID");
+  public static final String BASE_LOCATION = 
System.getenv("INTEGRATION_TEST_ADLS_PATH");
 
   @Override
   protected StorageConfigInfo getStorageConfigInfo() {
     return AzureStorageConfigInfo.builder()
         .setTenantId(TENANT_ID)
         .setStorageType(StorageConfigInfo.StorageTypeEnum.AZURE)
-        .setAllowedLocations(List.of(BASE_LOCATION))
+        .setAllowedLocations(
+            List.of(
+                StorageUtil.concatFilePrefixes(BASE_LOCATION, 
POLARIS_IT_SUBDIR, File.separator)))
         .build();
   }
 
   @Override
-  protected boolean shouldSkip() {
-    return Stream.of(BASE_LOCATION, 
TENANT_ID).anyMatch(Strings::isNullOrEmpty);
+  protected String getCustomMetadataLocationDir() {
+    return StorageUtil.concatFilePrefixes(BASE_LOCATION, POLARIS_IT_SUBDIR, 
File.separator);

Review Comment:
   Shouldn't this be:
   
   ```suggestion
       return StorageUtil.concatFilePrefixes(BASE_LOCATION, 
POLARIS_IT_CUSTOM_SUBDIR, File.separator);
   ```



##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java:
##########
@@ -86,6 +86,8 @@ public abstract class PolarisRestCatalogViewIntegrationBase 
extends ViewCatalogT
   private static PolarisApiEndpoints endpoints;
   private static PolarisClient client;
   private static ManagementApi managementApi;
+  protected static final String POLARIS_IT_SUBDIR = "polaris_it";
+  protected static final String POLARIS_IT_CUSTOM_SUBDIR = "polaris_it_custom";

Review Comment:
   This constant seems unused.



##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java:
##########
@@ -188,33 +182,89 @@ protected boolean overridesRequestedLocation() {
     return true;
   }
 
-  @Override
+  protected String getCustomMetadataLocationDir() {
+    return "";
+  }

Review Comment:
   ```suggestion
     protected abstract String getCustomMetadataLocationDir();
   ```



-- 
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]

Reply via email to