je-ik commented on a change in pull request #15082:
URL: https://github.com/apache/beam/pull/15082#discussion_r665543945
##########
File path:
sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/ExpansionService.java
##########
@@ -462,6 +488,9 @@ default InputT createInput(Pipeline p, Map<String,
PCollection<?>> inputs) {
throw new RuntimeException(exn);
}
}));
+
+
SplittableParDo.convertReadBasedSplittableDoFnsToPrimitiveReadsIfNecessary(pipeline);
Review comment:
The `convertReadBasedSplittableDoFnsToPrimitiveReadsIfNecessary` already
contains this check. Actually, it has the 'canonical' implementation of it - it
tests both `use_deprecated_read` and `beam_fn_api_use_deprecated_read` (and can
be overridden by `use_sdf_read`). I'd have to reimplement this check or else I
would change the semantics.
##########
File path:
sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/ExpansionService.java
##########
@@ -524,4 +554,15 @@ public static void main(String[] args) throws Exception {
server.start();
server.awaitTermination();
}
+
+ private static class NoOpRunner extends PipelineRunner<PipelineResult> {
+ public static NoOpRunner fromOptions(PipelineOptions opts) {
+ return new NoOpRunner();
+ }
+
Review comment:
I'd say, that the expansion process is tested in
https://github.com/apache/beam/blob/efc9b7fd8d4f7cedf006911d86b4467f40812046/sdks/java/expansion-service/src/test/java/org/apache/beam/sdk/expansion/service/ExpansionServiceTest.java#L120
The main point is that the expansion cannot call `run` method, as that
really does not make sense in this context. It would simplify things in the
context of ExpansionService, if the runner can be `null`, but that seems to be
the case only for ExpansionService, and I'd prefer to keep the validations as
they are. Maybe we can rename NoOpRunner to ThrowingRunner? Or
NotRunnableRunner?
--
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]
