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 2020/04/28 17:11:21 UTC

[GitHub] [beam] robertwb opened a new pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

robertwb opened a new pull request #11557:
URL: https://github.com/apache/beam/pull/11557


   This removes the need for a special jar_packages experiment, allowing expansion services to specify their own dependencies.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
   --- | --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
   Java | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
   Python | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
   XLang | --- | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) 
   Portable | --- | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   


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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624766720


   Run XVR_Flink PostCommit


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

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



[GitHub] [beam] chamikaramj commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-622489032


   Run XVR_Flink PostCommit


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

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



[GitHub] [beam] Hannah-Jiang commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
Hannah-Jiang commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-625332674


   > @Hannah-Jiang Is there a bug tracking these failures?
   
   Created https://github.com/apache/beam/pull/11630 for a fix.


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

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



[GitHub] [beam] Hannah-Jiang commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
Hannah-Jiang commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-625357723






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

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



[GitHub] [beam] mxm commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
mxm commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-625395042


   All green now.


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

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



[GitHub] [beam] chamikaramj commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-622489105


   Run XVR_Spark PostCommit


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

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



[GitHub] [beam] robertwb commented on a change in pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11557:
URL: https://github.com/apache/beam/pull/11557#discussion_r418405716



##########
File path: sdks/python/apache_beam/transforms/sql_test.py
##########
@@ -66,20 +66,8 @@ class SqlTransformTest(unittest.TestCase):
         --tests apache_beam.transforms.sql_test \\
         --test-pipeline-options="--runner=FlinkRunner"
   """
-  @staticmethod
-  def make_test_pipeline():
-    path_to_jar = subprocess_server.JavaJarServer.path_to_beam_jar(
-        ":sdks:java:extensions:sql:expansion-service:shadowJar")
-    test_pipeline = TestPipeline()
-    # TODO(BEAM-9238): Remove this when it's no longer needed for artifact
-    # staging.
-    test_pipeline.get_pipeline_options().view_as(DebugOptions).experiments = [
-        'jar_packages=' + path_to_jar

Review comment:
       Done.




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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624780180


   Run Python PreCommit


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

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



[GitHub] [beam] chamikaramj commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624801759


   Yeah, seems like x-lang test suites started failing few days back due to the pullLicenses issue.


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

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



[GitHub] [beam] chamikaramj commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624966377


   Seems like failure is due to https://github.com/apache/beam/pull/11548


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

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



[GitHub] [beam] mxm commented on a change in pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11557:
URL: https://github.com/apache/beam/pull/11557#discussion_r418609854



##########
File path: sdks/python/apache_beam/pipeline.py
##########
@@ -211,6 +211,8 @@ def __init__(self, runner=None, options=None, argv=None):
         experiments.append('beam_fn_api')
         self._options.view_as(DebugOptions).experiments = experiments
 
+    self.local_tempdir = tempfile.mkdtemp(prefix='beam-pipeline-temp')

Review comment:
       That's fine then, wasn't sure whether this is a unique directory.




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

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



[GitHub] [beam] chamikaramj commented on a change in pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on a change in pull request #11557:
URL: https://github.com/apache/beam/pull/11557#discussion_r418376845



##########
File path: sdks/python/apache_beam/transforms/sql_test.py
##########
@@ -66,20 +66,8 @@ class SqlTransformTest(unittest.TestCase):
         --tests apache_beam.transforms.sql_test \\
         --test-pipeline-options="--runner=FlinkRunner"
   """
-  @staticmethod
-  def make_test_pipeline():
-    path_to_jar = subprocess_server.JavaJarServer.path_to_beam_jar(
-        ":sdks:java:extensions:sql:expansion-service:shadowJar")
-    test_pipeline = TestPipeline()
-    # TODO(BEAM-9238): Remove this when it's no longer needed for artifact
-    # staging.
-    test_pipeline.get_pipeline_options().view_as(DebugOptions).experiments = [
-        'jar_packages=' + path_to_jar

Review comment:
       Can we update 'validate_runner_xlang_test.py' for Spark and Flink as well or do we need additional changes before that.
   jar_packages cannot be completely removed yet since Dataflow needs it.

##########
File path: sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/ExpansionService.java
##########
@@ -317,6 +321,14 @@ default InputT createInput(Pipeline p, Map<String, PCollection<?>> inputs) {
     Pipeline pipeline = Pipeline.create();
     ExperimentalOptions.addExperiment(
         pipeline.getOptions().as(ExperimentalOptions.class), "beam_fn_api");
+    List<String> classpathResources =
+        detectClassPathResourcesToStage(Environments.class.getClassLoader(), pipeline.getOptions());

Review comment:
       This assumes that the set of classpath resources required to execute Java SDK harness and execute the cross-langauge Java step to be the same as the set of classpath resources specified when starting up the expansion service, right ?
   
   I'm wondering if we can always rely on this. At least we should document this so that users are aware how to stage additional resources if needed.




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

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



[GitHub] [beam] robertwb commented on a change in pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11557:
URL: https://github.com/apache/beam/pull/11557#discussion_r418401987



##########
File path: sdks/java/expansion-service/src/main/java/org/apache/beam/sdk/expansion/service/ExpansionService.java
##########
@@ -317,6 +321,14 @@ default InputT createInput(Pipeline p, Map<String, PCollection<?>> inputs) {
     Pipeline pipeline = Pipeline.create();
     ExperimentalOptions.addExperiment(
         pipeline.getOptions().as(ExperimentalOptions.class), "beam_fn_api");
+    List<String> classpathResources =
+        detectClassPathResourcesToStage(Environments.class.getClassLoader(), pipeline.getOptions());

Review comment:
       Correct. This is a reasonable default (and the provider of an expansion service can always arrange that it is so), but we'll probably eventually want a way to customize this. 




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

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



[GitHub] [beam] mxm commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
mxm commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-622450697


   > Good point about needing to remove Kafka, etc. for Flink. (Not sure how that would interact with its use via an embedded environment; I'll let that be a later PR.)
   
   I think the embedded environment should have all the required dependencies in the classpath, similarly to how the expansion service has them at expansion time.


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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624963926


   @Hannah-Jiang Is there a bug tracking these failures?


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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624795306


   Run XVR_Flink PostCommit


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

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



[GitHub] [beam] chamikaramj commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624967725


   Created https://issues.apache.org/jira/browse/BEAM-9913


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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624795381


   Run XVR_Spark PostCommit


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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-620740988


   R: @mxm 
   CC: @chamikaramj @ihji 


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

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



[GitHub] [beam] mxm commented on a change in pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11557:
URL: https://github.com/apache/beam/pull/11557#discussion_r418060745



##########
File path: sdks/python/apache_beam/utils/subprocess_server.py
##########
@@ -98,7 +98,9 @@ def log_stdout():
         t.daemon = True
         t.start()
         wait_secs = .1
-        channel = grpc.insecure_channel(endpoint)
+        channel_options = [("grpc.max_receive_message_length", -1),
+                           ("grpc.max_send_message_length", -1)]

Review comment:
       Was this necessary to transfer the artifacts?

##########
File path: sdks/python/apache_beam/transforms/external.py
##########
@@ -317,13 +317,17 @@ def expand(self, pvalueish):
         transform=transform_proto)
 
     with self._service() as service:
+      print(type(service))

Review comment:
       Please remove or replace with logging

##########
File path: runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/DefaultArtifactResolver.java
##########
@@ -62,6 +63,24 @@ public void register(ResolutionFn fn) {
     fns.add(fn);
   }
 
+  @Override
+  public List<RunnerApi.ArtifactInformation> resolveArtifacts(
+      List<RunnerApi.ArtifactInformation> artifacts) {
+    for (ResolutionFn fn : Lists.reverse(fns)) {

Review comment:
       Is the resolution order documented somewhere? 

##########
File path: sdks/python/apache_beam/pipeline.py
##########
@@ -211,6 +211,8 @@ def __init__(self, runner=None, options=None, argv=None):
         experiments.append('beam_fn_api')
         self._options.view_as(DebugOptions).experiments = experiments
 
+    self.local_tempdir = tempfile.mkdtemp(prefix='beam-pipeline-temp')

Review comment:
       Should this be prefixed by something pipeline-specific?




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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624967141


   Things are passing locally (e.g. https://scans.gradle.com/s/m4g4v2flilkm6 ) but I would like to make sure Jenkins is happy so I'll hold off a bit more hoping we can get this resolved. 


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

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



[GitHub] [beam] chamikaramj commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624130608






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

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



[GitHub] [beam] robertwb commented on a change in pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11557:
URL: https://github.com/apache/beam/pull/11557#discussion_r418348695



##########
File path: sdks/python/apache_beam/utils/subprocess_server.py
##########
@@ -98,7 +98,9 @@ def log_stdout():
         t.daemon = True
         t.start()
         wait_secs = .1
-        channel = grpc.insecure_channel(endpoint)
+        channel_options = [("grpc.max_receive_message_length", -1),
+                           ("grpc.max_send_message_length", -1)]

Review comment:
       Yep, the default block size (4MB) produced messages just over the default grpc limits (4MB). I figured setting these here was safer, as the message size is chosen by the other side. We have similar settings elsewhere (e.g. on the data channel). 

##########
File path: sdks/python/apache_beam/pipeline.py
##########
@@ -211,6 +211,8 @@ def __init__(self, runner=None, options=None, argv=None):
         experiments.append('beam_fn_api')
         self._options.view_as(DebugOptions).experiments = experiments
 
+    self.local_tempdir = tempfile.mkdtemp(prefix='beam-pipeline-temp')

Review comment:
       Did you have something in mind? I don't think there's a good name here we can use. The system will of course ensure this directory is unique per pipeline. 

##########
File path: sdks/python/apache_beam/transforms/external.py
##########
@@ -317,13 +317,17 @@ def expand(self, pvalueish):
         transform=transform_proto)
 
     with self._service() as service:
+      print(type(service))

Review comment:
       Oops. Removed.

##########
File path: runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/DefaultArtifactResolver.java
##########
@@ -62,6 +63,24 @@ public void register(ResolutionFn fn) {
     fns.add(fn);
   }
 
+  @Override
+  public List<RunnerApi.ArtifactInformation> resolveArtifacts(
+      List<RunnerApi.ArtifactInformation> artifacts) {
+    for (ResolutionFn fn : Lists.reverse(fns)) {

Review comment:
       Yes, this is documented in the class's docstring. 




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

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



[GitHub] [beam] chamikaramj commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-625639303


   Thanks. Merging.


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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624795220


   Failure in sdks:java:container:pullLicenses


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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-622441649


   Run Java PreCommit


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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-622441715


   Run Python PreCommit


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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-622304615






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

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



[GitHub] [beam] robertwb commented on pull request #11557: [BEAM-9845] Stage artifacts over expansion service.

Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #11557:
URL: https://github.com/apache/beam/pull/11557#issuecomment-624188539


   Thanks. Those suites passed locally, I'll look into what's going on here. 


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

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