Abacn commented on code in PR #39758:
URL: https://github.com/apache/beam/pull/39758#discussion_r3825212621
##########
sdks/java/io/delta/src/test/java/org/apache/beam/sdk/io/delta/DeltaIOIT.java:
##########
@@ -302,6 +302,116 @@ public void testReadDeltaLakeTable() {
readPipeline.run().waitUntilFinish();
}
+ @Test
+ public void testReadDeltaLakeTableAtTimestamp() throws Exception {
+ ExperimentalOptions options =
readPipeline.getOptions().as(ExperimentalOptions.class);
+ ExperimentalOptions.addExperiment(options, "use_runner_v2");
+
+ Map<String, String> hadoopConfig = new HashMap<>();
+ hadoopConfig.put("fs.gs.impl",
"com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystem");
Review Comment:
There are duplicated codes. Can we move the creation of hadoop Configuration
into a helper function?
##########
sdks/java/io/delta/src/test/java/org/apache/beam/sdk/io/delta/DeltaIOIT.java:
##########
@@ -302,6 +302,116 @@ public void testReadDeltaLakeTable() {
readPipeline.run().waitUntilFinish();
}
+ @Test
Review Comment:
Is DeltaIOIT exercised by any GHA tests? Checking
https://github.com/apache/beam/blob/master/.github/workflows/beam_PreCommit_Java_Delta_IO_Direct.yml
it only runs `:sdks:java:io:delta:build` not
`:sdks:java:io:delta:integrationTest`
##########
sdks/java/io/delta/src/test/java/org/apache/beam/sdk/io/delta/DeltaIOIT.java:
##########
@@ -302,6 +302,116 @@ public void testReadDeltaLakeTable() {
readPipeline.run().waitUntilFinish();
}
+ @Test
+ public void testReadDeltaLakeTableAtTimestamp() throws Exception {
+ ExperimentalOptions options =
readPipeline.getOptions().as(ExperimentalOptions.class);
+ ExperimentalOptions.addExperiment(options, "use_runner_v2");
+
+ Map<String, String> hadoopConfig = new HashMap<>();
+ hadoopConfig.put("fs.gs.impl",
"com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystem");
+ hadoopConfig.put(
+ "fs.AbstractFileSystem.gs.impl",
"com.google.cloud.hadoop.fs.gcs.GoogleHadoopFS");
+ hadoopConfig.put("fs.gs.auth.type", "APPLICATION_DEFAULT");
+ String project =
+ readPipeline
+ .getOptions()
+ .as(org.apache.beam.sdk.extensions.gcp.options.GcpOptions.class)
+ .getProject();
+ if (project != null) {
+ hadoopConfig.put("fs.gs.project.id", project);
+ }
+
+ org.apache.hadoop.conf.Configuration conf = new
org.apache.hadoop.conf.Configuration();
+ for (Map.Entry<String, String> entry : hadoopConfig.entrySet()) {
+ conf.set(entry.getKey(), entry.getValue());
+ }
+ Engine engine = DefaultEngine.create(conf);
+
+ // Wait briefly to ensure timestamp is after version 0 commit
+ Thread.sleep(1000);
Review Comment:
sleep is generally unreliable especially on CI. Consider using a reliable
way to detect initial setup is finished, e.g. poll the file system?
##########
sdks/java/io/delta/src/test/java/org/apache/beam/sdk/io/delta/DeltaIOTest.java:
##########
@@ -109,6 +109,185 @@ public void testReadRowsNullDefaults() {
Assert.assertNull(readRows.getHadoopConfig());
}
+ @Test
+ public void testReadRowsBothVersionAndTimestampThrows() {
+ org.apache.beam.sdk.Pipeline p = org.apache.beam.sdk.Pipeline.create();
+ IllegalArgumentException exception =
+ Assert.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ p.apply(
+ DeltaIO.readRows()
+ .from("/path/to/table")
+ .withVersion(0L)
+ .withTimestamp("2026-05-20T15:43:26Z")));
+ Assert.assertTrue(exception.getMessage().contains("Cannot set both version
and timestamp."));
+ }
+
+ @Test
+ public void testReadRowsAtVersion() throws Exception {
+ File tableDir = tempFolder.newFolder("delta-table-read-version");
+ Engine engine = DefaultEngine.create(new
org.apache.hadoop.conf.Configuration());
+
+ Schema schema = Schema.builder().addField("name",
Schema.FieldType.STRING).build();
Review Comment:
Same here, consider move repeated test data setup fixtures into a helper
function
--
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]
