You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/07/13 21:20:33 UTC

[GitHub] [beam] chamikaramj commented on a diff in pull request #22004: [BEAM-22003] Allow merging consecutive external transforms in Java SDK

chamikaramj commented on code in PR #22004:
URL: https://github.com/apache/beam/pull/22004#discussion_r920523534


##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/External.java:
##########
@@ -76,25 +81,36 @@
   "nullness" // TODO(https://github.com/apache/beam/issues/20497)
 })
 public class External {
-  private static final String EXPANDED_TRANSFORM_BASE_NAME = "external";
+  protected static final String EXPANDED_TRANSFORM_BASE_NAME = "external";
   private static final String IMPULSE_PREFIX = "IMPULSE";
   private static AtomicInteger namespaceCounter = new AtomicInteger(0);
 
   private static final ExpansionServiceClientFactory DEFAULT =
       DefaultExpansionServiceClientFactory.create(
           endPoint -> ManagedChannelBuilder.forTarget(endPoint.getUrl()).usePlaintext().build());
 
-  private static int getFreshNamespaceIndex() {
+  public static int getFreshNamespaceIndex() {
     return namespaceCounter.getAndIncrement();
   }
 
   public static <InputT extends PInput, OutputT>
       SingleOutputExpandableTransform<InputT, OutputT> of(
           String urn, byte[] payload, String endpoint) {
-    Endpoints.ApiServiceDescriptor apiDesc =
-        Endpoints.ApiServiceDescriptor.newBuilder().setUrl(endpoint).build();
     return new SingleOutputExpandableTransform<>(
-        urn, payload, apiDesc, DEFAULT, getFreshNamespaceIndex(), ImmutableMap.of());
+        ImmutableList.of(ExpansionInfo.create(urn, payload, endpoint, getFreshNamespaceIndex())),
+        DEFAULT,
+        ImmutableMap.of());
+  }
+
+  public static <InputT extends PInput, OutputT>
+      SingleOutputExpandableTransform<InputT, OutputT> of(

Review Comment:
   Why do we need this syntax ? Beam transform composing is done through building composite and "consecutive" external transforms would imply consecutive "apply" methods to me. 



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org