gemini-code-assist[bot] commented on code in PR #38490:
URL: https://github.com/apache/beam/pull/38490#discussion_r3236727246
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/SimpleDoFnRunner.java:
##########
@@ -1063,6 +1105,19 @@ public <T> void outputWithTimestamp(TupleTag<T> tag, T
output, Instant timestamp
tag, output, timestamp, Collections.singleton(window()),
PaneInfo.NO_FIRING);
}
+ @Override
+ public <T> void outputWithKind(TupleTag<T> tag, T output, ValueKind kind) {
+ checkTimestamp(timestamp(), timestamp);
Review Comment:
The variable `timestamp` is not defined in the scope of the `outputWithKind`
method, which will lead to a compilation error. This appears to be a copy-paste
error from the `outputWithTimestamp` method. Since the output timestamp is
implicitly the context's firing timestamp (`timestamp()`), this check is
redundant and should be removed.
##########
runners/core-java/src/main/java/org/apache/beam/runners/core/SimpleDoFnRunner.java:
##########
@@ -1352,6 +1417,19 @@ public <T> void outputWithTimestamp(TupleTag<T> tag, T
output, Instant timestamp
tag, output, timestamp, Collections.singleton(window()),
PaneInfo.NO_FIRING);
}
+ @Override
+ public <T> void outputWithKind(TupleTag<T> tag, T output, ValueKind kind) {
+ checkTimestamp(this.timestamp, timestamp);
Review Comment:
The variable `timestamp` is not defined in this method, likely due to a
copy-paste error from `outputWithTimestamp`. In this context, the output
timestamp is fixed to `this.timestamp`, making this check redundant. It should
be removed to avoid compilation issues.
##########
runners/google-cloud-dataflow-java/src/test/java/org/apache/beam/runners/dataflow/DataflowRunnerTest.java:
##########
@@ -2777,4 +2785,133 @@ public void process() {}
}));
p.run();
}
+
+ @Test
+ @Category({ValidatesRunner.class})
+ public void testValueKindParameterAndOutputWithKind() {
+ boolean isRunnerV2 = false;
+ @Nullable
+ List<String> experiments =
+
pipeline.getOptions().as(DataflowPipelineOptions.class).getExperiments();
+ if (experiments != null
+ && (experiments.contains("use_unified_worker") ||
experiments.contains("use_runner_v2"))) {
+ isRunnerV2 = true;
+ }
+ // Skipp runner v2 because its Create uses a splittable DoFn, which
contains a shuffle.
+ // ValueKind is not supported in Dataflow shuffle yet
+ assumeFalse(isRunnerV2);
Review Comment:
The method `assumeFalse` is used here but the required static import `import
static org.junit.Assume.assumeFalse;` is missing from the imports section,
which will cause a build failure in the tests.
--
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]
