Skip to content

Commit 76c838c

Browse files
cbb330cbb330
committed
chore: cleanup Gradle and TestBase code
- Remove legacy openHouseFixturesCoordinate approach with exclusions - Use openhouseCompatibilityRuntime with transitive=false (cleaner) - Remove hardcoded SNAPSHOT coordinate from gradle.properties - Simplify TestBase.loadSparkSessionProvider using ServiceLoader.findFirst() - Remove redundant system property handling from spark/v3.5/build.gradle
1 parent 3bf160d commit 76c838c

File tree

3 files changed

+24
-133
lines changed

3 files changed

+24
-133
lines changed

‎gradle.properties‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,5 @@ systemProp.defaultSparkVersions=3.5
2424
systemProp.knownSparkVersions=3.3,3.4,3.5
2525
systemProp.defaultScalaVersion=2.12
2626
systemProp.knownScalaVersions=2.12,2.13
27-
systemProp.openHouseCompatibilityCoordinate=com.linkedin.openhouse:tables-test-fixtures_2.12:0.0.1-SNAPSHOT:uber
2827
org.gradle.parallel=true
2928
org.gradle.jvmargs=-Xmx1024m

‎spark/v3.5/build.gradle‎

Lines changed: 3 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -19,37 +19,6 @@
1919

2020
String sparkMajorVersion = '3.5'
2121
String scalaVersion = System.getProperty("scalaVersion") != null ? System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
22-
String openHouseFixturesCoordinate = gradle.startParameter.systemPropertiesArgs.get("openhouseFixturesCoordinate")
23-
if (openHouseFixturesCoordinate != null) {
24-
// The OpenHouse fixture is published as an executable Spring Boot jar by default. When consumers
25-
// use it as a library we need the "-lib" (a.k.a. "uber") classifier that exposes the plain class
26-
// files on the classpath. Automatically add the classifier when the caller only provides
27-
// the typical groupId:artifactId:version coordinate.
28-
int colonCount = openHouseFixturesCoordinate.count(":")
29-
if (colonCount == 2) {
30-
openHouseFixturesCoordinate = openHouseFixturesCoordinate + ":uber"
31-
}
32-
}
33-
34-
def sparkSessionProviderValue =
35-
gradle.startParameter.systemPropertiesArgs.get("iceberg.test.spark.session.provider")
36-
def catalogProviderValue =
37-
gradle.startParameter.systemPropertiesArgs.get("iceberg.test.catalog.provider")
38-
39-
def skipDefaultCatalogsValue =
40-
gradle.startParameter.systemPropertiesArgs.get("iceberg.test.catalog.skip.defaults")
41-
42-
if (openHouseFixturesCoordinate != null) {
43-
if (sparkSessionProviderValue == null || sparkSessionProviderValue.isEmpty()) {
44-
sparkSessionProviderValue = 'org.apache.iceberg.spark.openhouse.OpenHouseSparkITestProvider'
45-
}
46-
if (catalogProviderValue == null || catalogProviderValue.isEmpty()) {
47-
catalogProviderValue = 'org.apache.iceberg.spark.openhouse.OpenHouseSparkITestProvider'
48-
}
49-
if (skipDefaultCatalogsValue == null || skipDefaultCatalogsValue.isEmpty()) {
50-
skipDefaultCatalogsValue = 'true'
51-
}
52-
}
5322

5423
def sparkProjects = [
5524
project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}"),
@@ -147,21 +116,8 @@ project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") {
147116
testImplementation libs.sqlite.jdbc
148117
testImplementation libs.awaitility
149118

150-
if (openHouseFixturesCoordinate != null) {
151-
testRuntimeOnly(openHouseFixturesCoordinate) {
152-
exclude group: 'org.apache.iceberg'
153-
exclude group: 'com.linkedin.iceberg'
154-
exclude group: 'org.apache.logging.log4j', module: 'log4j-slf4j2-impl'
155-
exclude group: 'org.apache.logging.log4j', module: 'log4j-to-slf4j'
156-
exclude group: 'com.fasterxml.jackson.core'
157-
exclude group: 'com.fasterxml.jackson.module'
158-
exclude group: 'com.fasterxml.jackson.datatype'
159-
exclude group: 'com.fasterxml.jackson.dataformat'
160-
}
161-
testRuntimeOnly project(':iceberg-aws')
162-
testRuntimeOnly 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.13.4'
163-
}
164-
119+
// OpenHouse compatibility testing dependencies - uber jar with transitive=false
120+
// avoids classpath conflicts since the jar is self-contained with shaded dependencies
165121
openhouseCompatibilityRuntime(rootProject.openHouseCompatibilityCoordinate) {
166122
transitive = false
167123
}
@@ -171,32 +127,13 @@ project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") {
171127
openhouseCompatibilityRuntime 'com.zaxxer:HikariCP:4.0.3'
172128
}
173129

174-
if (openHouseFixturesCoordinate != null) {
175-
configurations {
176-
testRuntimeClasspath {
177-
exclude group: 'org.apache.logging.log4j', module: 'log4j-slf4j2-impl'
178-
}
179-
}
180-
}
181-
182130
test {
183131
useJUnitPlatform()
184132
}
185133

186134
tasks.withType(Test).configureEach {
187135
// Vectorized reads need more memory
188136
maxHeapSize '2560m'
189-
190-
if (sparkSessionProviderValue != null && !sparkSessionProviderValue.isEmpty()) {
191-
systemProperty "iceberg.test.spark.session.provider", sparkSessionProviderValue
192-
}
193-
194-
if (catalogProviderValue != null && !catalogProviderValue.isEmpty()) {
195-
systemProperty "iceberg.test.catalog.provider", catalogProviderValue
196-
}
197-
if (skipDefaultCatalogsValue != null && !skipDefaultCatalogsValue.isEmpty()) {
198-
systemProperty "iceberg.test.catalog.skip.defaults", skipDefaultCatalogsValue
199-
}
200137
}
201138

202139
apply from: "$projectDir/../../openhouse.gradle"
@@ -300,12 +237,6 @@ project(":iceberg-spark:iceberg-spark-runtime-${sparkMajorVersion}_${scalaVersio
300237
exclude group: 'org.scala-lang'
301238
exclude group: 'org.scala-lang.modules'
302239
}
303-
304-
integrationImplementation {
305-
// Log4j routing is handled via slf4j-simple/log4j-to-slf4j. Drop Spark's slf4j2 bridge to
306-
// avoid Log4j enforcing mutual exclusivity at runtime.
307-
exclude group: 'org.apache.logging.log4j', module: 'log4j-slf4j2-impl'
308-
}
309240
}
310241

311242
dependencies {
@@ -338,15 +269,6 @@ project(":iceberg-spark:iceberg-spark-runtime-${sparkMajorVersion}_${scalaVersio
338269
integrationImplementation project(path: ':iceberg-hive-metastore', configuration: 'testArtifacts')
339270
integrationImplementation project(path: ":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}", configuration: 'testArtifacts')
340271
integrationImplementation project(path: ":iceberg-spark:iceberg-spark-extensions-${sparkMajorVersion}_${scalaVersion}", configuration: 'testArtifacts')
341-
if (openHouseFixturesCoordinate != null) {
342-
integrationImplementation(openHouseFixturesCoordinate) {
343-
// The fixtures already include Iceberg bits transitively; drop them to avoid conflicts
344-
exclude group: 'org.apache.iceberg'
345-
exclude group: 'com.linkedin.iceberg'
346-
// Avoid conflicting SLF4J bridges; Iceberg already routes logging through log4j-to-slf4j
347-
exclude group: 'org.apache.logging.log4j', module: 'log4j-slf4j2-impl'
348-
}
349-
}
350272
// Not allowed on our classpath, only the runtime jar is allowed
351273
integrationCompileOnly project(":iceberg-spark:iceberg-spark-extensions-${sparkMajorVersion}_${scalaVersion}")
352274
integrationCompileOnly project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}")
@@ -394,7 +316,7 @@ project(":iceberg-spark:iceberg-spark-runtime-${sparkMajorVersion}_${scalaVersio
394316
description = "Test Spark3 Runtime Jar against Spark ${sparkMajorVersion}"
395317
group = "verification"
396318
if (project.hasProperty('extraJvmArgs')) {
397-
jvmArgs += project.property('extraJvmArgs')
319+
jvmArgs += project.property('extraJvmArgs')
398320
}
399321
testClassesDirs = sourceSets.integration.output.classesDirs
400322
classpath = sourceSets.integration.runtimeClasspath + files(shadowJar.archiveFile.get().asFile.path)
@@ -403,19 +325,6 @@ project(":iceberg-spark:iceberg-spark-runtime-${sparkMajorVersion}_${scalaVersio
403325
integrationTest.dependsOn shadowJar
404326
check.dependsOn integrationTest
405327

406-
tasks.withType(Test).configureEach {
407-
if (sparkSessionProviderValue != null && !sparkSessionProviderValue.isEmpty()) {
408-
systemProperty "iceberg.test.spark.session.provider", sparkSessionProviderValue
409-
}
410-
411-
if (catalogProviderValue != null && !catalogProviderValue.isEmpty()) {
412-
systemProperty "iceberg.test.catalog.provider", catalogProviderValue
413-
}
414-
if (skipDefaultCatalogsValue != null && !skipDefaultCatalogsValue.isEmpty()) {
415-
systemProperty "iceberg.test.catalog.skip.defaults", skipDefaultCatalogsValue
416-
}
417-
}
418-
419328
jar {
420329
enabled = false
421330
}

‎spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestBase.java‎

Lines changed: 21 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,12 @@
2727
import java.nio.file.Files;
2828
import java.nio.file.Path;
2929
import java.nio.file.Paths;
30-
import java.util.Iterator;
3130
import java.util.List;
3231
import java.util.Map;
32+
import java.util.ServiceLoader;
3333
import java.util.TimeZone;
3434
import java.util.concurrent.TimeoutException;
3535
import java.util.concurrent.atomic.AtomicReference;
36-
import java.util.ServiceLoader;
3736
import org.apache.hadoop.hive.conf.HiveConf;
3837
import org.apache.iceberg.CatalogUtil;
3938
import org.apache.iceberg.ContentFile;
@@ -117,60 +116,44 @@ public static void stopMetastoreAndSpark() throws Exception {
117116
}
118117
}
119118

120-
protected static SparkSession.Builder createDefaultSparkBuilder(HiveConf hiveConf) {
119+
protected static SparkSession.Builder createDefaultSparkBuilder(HiveConf conf) {
121120
return SparkSession.builder()
122121
.master("local[2]")
123122
.config(SQLConf.PARTITION_OVERWRITE_MODE().key(), "dynamic")
124-
.config("spark.hadoop." + METASTOREURIS.varname, hiveConf.get(METASTOREURIS.varname))
123+
.config("spark.hadoop." + METASTOREURIS.varname, conf.get(METASTOREURIS.varname))
125124
.config("spark.sql.legacy.respectNullabilityInTextDatasetConversion", "true")
126125
.enableHiveSupport();
127126
}
128127

129-
protected static SparkSession buildSparkSession(
130-
SparkSession.Builder builder, HiveConf hiveConf) {
128+
protected static SparkSession buildSparkSession(SparkSession.Builder builder, HiveConf conf) {
131129
sparkSessionProvider = loadSparkSessionProvider();
132-
if (sparkSessionProvider != null) {
133-
try {
134-
sparkSessionProvider.beforeAll();
135-
SparkSession provided = sparkSessionProvider.createSparkSession(builder, hiveConf);
136-
return provided != null ? provided : builder.getOrCreate();
137-
} catch (Exception e) {
138-
throw new RuntimeException("Failed to initialize custom Spark session provider", e);
139-
}
140-
} else {
130+
if (sparkSessionProvider == null) {
141131
return builder.getOrCreate();
142132
}
133+
134+
try {
135+
sparkSessionProvider.beforeAll();
136+
SparkSession session = sparkSessionProvider.createSparkSession(builder, conf);
137+
return session != null ? session : builder.getOrCreate();
138+
} catch (Exception e) {
139+
throw new RuntimeException("Failed to initialize custom Spark session provider", e);
140+
}
143141
}
144142

145143
private static TestSparkSessionProvider loadSparkSessionProvider() {
144+
// Check system property first for explicit override
146145
String providerClass = System.getProperty(SPARK_SESSION_PROVIDER_PROPERTY);
147146
if (providerClass != null && !providerClass.isEmpty()) {
148-
return instantiateSparkSessionProvider(providerClass);
149-
}
150-
151-
try {
152-
ServiceLoader<TestSparkSessionProvider> loader =
153-
ServiceLoader.load(TestSparkSessionProvider.class);
154-
Iterator<TestSparkSessionProvider> iterator = loader.iterator();
155-
if (iterator.hasNext()) {
156-
return iterator.next();
147+
try {
148+
return (TestSparkSessionProvider)
149+
Class.forName(providerClass).getDeclaredConstructor().newInstance();
150+
} catch (ReflectiveOperationException e) {
151+
throw new RuntimeException("Failed to instantiate " + providerClass, e);
157152
}
158-
} catch (Exception e) {
159-
System.err.println(
160-
"Failed to initialize ServiceLoader for TestSparkSessionProvider: " + e.getMessage());
161153
}
162154

163-
return null;
164-
}
165-
166-
private static TestSparkSessionProvider instantiateSparkSessionProvider(String className) {
167-
try {
168-
return (TestSparkSessionProvider)
169-
Class.forName(className).getDeclaredConstructor().newInstance();
170-
} catch (Exception e) {
171-
throw new RuntimeException(
172-
"Failed to instantiate TestSparkSessionProvider: " + className, e);
173-
}
155+
// Fall back to SPI discovery
156+
return ServiceLoader.load(TestSparkSessionProvider.class).findFirst().orElse(null);
174157
}
175158

176159
protected long waitUntilAfter(long timestampMillis) {

0 commit comments

Comments
 (0)