Skip to content

Spark: Improvements around test initialization #10131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
amogh-jahagirdar merged 1 commit into from
Apr 12, 2024
Merged

Spark: Improvements around test initialization #10131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...park-extensions/src/test/java/org/apache/iceberg/spark/extensions/ExtensionsTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.spark.CatalogTestBase;
import org.apache.iceberg.spark.TestBase;
import org.apache.spark.api.java.JavaSparkContext;
import org.apache.spark.sql.SparkSession;
import org.apache.spark.sql.internal.SQLConf;
import org.junit.jupiter.api.BeforeAll;
Expand Down Expand Up @@ -57,6 +58,8 @@ public static void startMetastoreAndSpark() {
.enableHiveSupport()
.getOrCreate();

TestBase.sparkContext = JavaSparkContext.fromSparkContext(spark.sparkContext());
Copy link
Copy Markdown
Contributor Author

@nastra nastra Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestBase.spark is created in the line above, so we should probably configure the spark context appropriately here


TestBase.catalog =
(HiveCatalog)
CatalogUtil.loadCatalog(
Expand Down
4 changes: 0 additions & 4 deletions spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/CatalogTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@
*/
package org.apache.iceberg.spark;

import java.nio.file.Path;
import org.apache.iceberg.ParameterizedTestExtension;
import org.apache.iceberg.Parameters;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.io.TempDir;

@ExtendWith(ParameterizedTestExtension.class)
public abstract class CatalogTestBase extends TestBaseWithCatalog {
Expand All @@ -48,6 +46,4 @@ protected static Object[][] parameters() {
}
};
}

@TempDir protected Path temp;
Copy link
Copy Markdown
Contributor Author

@nastra nastra Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's already a temp variable available in the super class (TestBaseWithCatalog), so we don't need to have 2 different ones

}
2 changes: 1 addition & 1 deletion spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestBaseWithCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public static void dropWarehouse() throws IOException {
}
}

@TempDir protected File temp;
@TempDir protected java.nio.file.Path temp;
Copy link
Copy Markdown
Contributor

@amogh-jahagirdar amogh-jahagirdar Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nit, but while we're doing this: I find it easier to read if we don't use the fully qualified Path name here, rather use the fullly qualified name for the hadoop Path in dropWarehouse.


@Parameter(index = 0)
protected String catalogName;
Expand Down
4 changes: 2 additions & 2 deletions spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkExecutorCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,13 @@ private DeleteFile writeEqDeletes(Table table, String col, Object... values) thr
deletes.add(delete.copy(col, value));
}

OutputFile out = Files.localOutput(new File(temp, "eq-deletes-" + UUID.randomUUID()));
OutputFile out = Files.localOutput(new File(temp.toFile(), "eq-deletes-" + UUID.randomUUID()));
return FileHelpers.writeDeleteFile(table, out, null, deletes, deleteSchema);
}

private Pair<DeleteFile, CharSequenceSet> writePosDeletes(
Table table, List<Pair<CharSequence, Long>> deletes) throws IOException {
OutputFile out = Files.localOutput(new File(temp, "pos-deletes-" + UUID.randomUUID()));
OutputFile out = Files.localOutput(new File(temp.toFile(), "pos-deletes-" + UUID.randomUUID()));
return FileHelpers.writeDeleteFile(table, out, null, deletes);
}

Expand Down