[GitHub] [beam] chamikaramj commented on a diff in pull request #25222: Update PythonMap transform to accept extra packages

["chamikaramj (via GitHub)" <gi...@apache.org>]

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


##########
sdks/java/extensions/python/src/main/java/org/apache/beam/sdk/extensions/python/transforms/PythonMap.java:
##########
@@ -59,13 +63,34 @@ public PythonMap<InputT, OutputT> withExpansionService(String expansionService)
     return this;
   }
 
+  /**
+   * Specifies any extra Pypi packages required by the RunInference model handler.
+   *
+   * <p>This should only be specified when using the default expansion service, i.e. when not using
+   * {@link #withExpansionService(String)} to provide an expansion service.
+   *
+   * <p>For model handlers provided by Beam Python SDK, the implementation will automatically try to
+   * infer correct packages needed, so this may be omitted.

Review Comment:
   Good catch. Done.



##########
sdks/java/extensions/python/src/main/java/org/apache/beam/sdk/extensions/python/transforms/PythonMap.java:
##########
@@ -59,13 +63,34 @@ public PythonMap<InputT, OutputT> withExpansionService(String expansionService)
     return this;
   }
 
+  /**
+   * Specifies any extra Pypi packages required by the RunInference model handler.
+   *
+   * <p>This should only be specified when using the default expansion service, i.e. when not using
+   * {@link #withExpansionService(String)} to provide an expansion service.
+   *
+   * <p>For model handlers provided by Beam Python SDK, the implementation will automatically try to
+   * infer correct packages needed, so this may be omitted.
+   *
+   * @param extraPackages a list of PyPi packages. May include the version.
+   * @return A {@link PythonMap} with extra packages.
+   */
+  public PythonMap<InputT, OutputT> withExtraPackages(List<String> extraPackages) {
+    if (!this.extraPackages.isEmpty()) {

Review Comment:
   We already perform such a check at PythonExternalTransform so don't think we need to repeat the check here.
   
   The check is to make sure that the expansion service is not specified BTW. withExtraPackages only works with the auto-started expansion services.
   
   https://github.com/apache/beam/blob/9fcb3a5b48a6db0dcf57f454d1d1eca10cf1c41b/sdks/java/extensions/python/src/main/java/org/apache/beam/sdk/extensions/python/PythonExternalTransform.java#L292



-- 
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