MonkeyCanCode commented on code in PR #4588:
URL: https://github.com/apache/polaris/pull/4588#discussion_r3338475533


##########
plugins/spark/v3.5/integration/src/intTest/java/org/apache/polaris/spark/quarkus/it/BundleJarSanityIT.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.spark.quarkus.it;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.quarkus.test.junit.QuarkusIntegrationTest;
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.polaris.core.admin.model.Catalog;
+import org.apache.polaris.core.admin.model.CatalogProperties;
+import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
+import org.apache.polaris.core.admin.model.PolarisCatalog;
+import org.apache.polaris.core.admin.model.StorageConfigInfo;
+import org.apache.polaris.service.it.env.ClientCredentials;
+import org.apache.polaris.service.it.env.ManagementApi;
+import org.apache.polaris.service.it.env.PolarisApiEndpoints;
+import org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+
+@QuarkusIntegrationTest
+@ExtendWith(PolarisIntegrationTestExtension.class)
+public class BundleJarSanityIT {
+
+  /**
+   * This test verifies that the Polaris Spark bundle jar can be loaded in a 
fresh JVM with only
+   * Spark dependencies, and be able to interact with the Polaris server.
+   */
+  @Test
+  void testBundleJarLoading(
+      @TempDir Path tempDir, PolarisApiEndpoints endpoints, ClientCredentials 
credentials)
+      throws Exception {
+    String bundleJarPath = System.getProperty("polaris.spark.bundle.jar");
+    assertThat(bundleJarPath)
+        .withFailMessage("polaris.spark.bundle.jar property not set")
+        .isNotNull();
+
+    File bundleJar = new File(bundleJarPath);
+    assertThat(bundleJar).exists();
+
+    try (PolarisManagementClient client = 
PolarisManagementClient.managementClient(endpoints)) {
+      String catalogName = client.newEntityName("bundle_test_catalog");
+      ManagementApi managementApi = client.managementApi(credentials);
+
+      String catalogBaseLocation = 
tempDir.resolve("catalog").toUri().toString();
+      FileStorageConfigInfo storageConfig =
+          FileStorageConfigInfo.builder()
+              .setStorageType(StorageConfigInfo.StorageTypeEnum.FILE)
+              .setAllowedLocations(List.of(catalogBaseLocation))
+              .build();
+      Catalog catalog =
+          PolarisCatalog.builder()
+              .setType(Catalog.TypeEnum.INTERNAL)
+              .setName(catalogName)
+              .setProperties(new CatalogProperties(catalogBaseLocation))
+              .setStorageConfigInfo(storageConfig)
+              .build();
+
+      managementApi.createCatalog(catalog);
+      try {
+        runIsolatedSparkCheck(
+            bundleJar, tempDir, catalogName, endpoints, 
client.obtainToken(credentials));
+      } finally {
+        managementApi.deleteCatalog(catalogName);
+      }
+    }
+  }
+
+  private void runIsolatedSparkCheck(
+      File bundleJar, Path tempDir, String catalogName, PolarisApiEndpoints 
endpoints, String token)
+      throws Exception {
+    // Filter the current classpath: drop polaris-spark / polaris-core so the 
bundle jar
+    // is the sole source of those classes; keep external jars (spark-sql, 
iceberg, etc.).
+    String[] parts = 
System.getProperty("java.class.path").split(File.pathSeparator);

Review Comment:
   I was think the real gap between the current regtest (under plugin/spark and 
not the one from project root directory) and integration is the "spark-shell 
--jar xxxx", which this approach is more close to real world simulation with a 
new process will try to mimics how a user actually deploy the jar. With the 
purposed route, I am worried we may run into classpath hell or hide some 
packaging bugs.
   
   If we think the purposed approach shouldn't be a concern with and be more 
preferred, I am fine with making the requested changes. 



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