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

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

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


##########
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:
   Should we add a check that `expansionService` is null and throw an early error if not? Is this a requirement for all uses of PythonMap or only when using it with RunInference?



##########
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:
   This documentation is specific to RunInference use case, may help to write a generalized one



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