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 2021/03/22 20:15:32 UTC

[GitHub] [beam] kennknowles opened a new pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

kennknowles opened a new pull request #14297:
URL: https://github.com/apache/beam/pull/14297


   These overrides were implicitly removed when we switched to using the original graph instead of the override graph. But Dataflow still needs these overrides.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [x] 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.
    - [x] Update `CHANGES.md` with noteworthy changes.
    - [x] 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 | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
   Java | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam
 .apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://ci-beam.a
 pache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam
 .apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
   XLang | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website | Whitespace | Typescript
   --- | --- | --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/be
 am_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/)
   Portable | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.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.
   
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   OK. After #14321 I think it could make sense to even separate different ITs with different quotas. We might also need to put a retry limit or something on HealthCareIO or timeout on the specific test cases. It should fail instead of timing out so we can get a gradle scan, etc.


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   run java 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] kennknowles closed pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

Posted by GitBox <gi...@apache.org>.
kennknowles closed pull request #14297:
URL: https://github.com/apache/beam/pull/14297


   


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   Actually because of BEAM-7716 and BEAM-7717 probably noone should use the custom PubsubIO.


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   Re-reading this code, it is a bad cherrypick and edit. The change does not do what it says. It may have been useful for fixing Java Postcommit before PJS was rolled back, but now it doesn't matter.


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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






-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   Linking to most recent failure for archive before I adjust the PR: https://ci-beam.apache.org/job/beam_PostCommit_Java_PR/637/


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   run java 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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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






-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   run java 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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   (but I am not trying to disable it in this PR, so I will check those experiments)


-- 
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] kennknowles commented on a change in pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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



##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -454,6 +454,17 @@ protected DataflowRunner(DataflowPipelineOptions options) {
     this.ptransformViewsWithNonDeterministicKeyCoders = new HashSet<>();
   }
 
+  /** For portable jobs, Dataflow still requires an override of the PubsubIO transforms. */
+  private List<PTransformOverride> getPortableOverrides() {
+    return ImmutableList.of(
+        PTransformOverride.of(

Review comment:
       Yes, good catch.




-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   Clarification: the last successful build of the `_PR` job was 3 hours and a bit. The actual last successful postcommit I think was GC'd. The last one retained was 1 hour 45 minutes.


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   run java 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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   On #14317 you make the good point that this may as well be merged there. So that is what I have 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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   Failure was not in Pubsub. This change really should work fine since we aren't even using portable job submission anymore.


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   run java 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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   You can see in https://ci-beam.apache.org/job/beam_PostCommit_Java_PR/631/ that this fixes the Pubsub postcommit. The other failure is the BigQuery storage test that is fixed in #14299 


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   CC: @tvalentyn 


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   The perfect situation would be to have zero overrides for runner v2 and do everything in the backend.
   
   For now, we keep just the new Pubsub override. Hopefully we are able to get to zero.


-- 
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] boyuanzz commented on a change in pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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



##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -454,6 +454,17 @@ protected DataflowRunner(DataflowPipelineOptions options) {
     this.ptransformViewsWithNonDeterministicKeyCoders = new HashSet<>();
   }
 
+  /** For portable jobs, Dataflow still requires an override of the PubsubIO transforms. */
+  private List<PTransformOverride> getPortableOverrides() {
+    return ImmutableList.of(
+        PTransformOverride.of(

Review comment:
       We should also check `hasExperiment(options, "enable_custom_pubsub_source")`

##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -454,6 +454,17 @@ protected DataflowRunner(DataflowPipelineOptions options) {
     this.ptransformViewsWithNonDeterministicKeyCoders = new HashSet<>();
   }
 
+  /** For portable jobs, Dataflow still requires an override of the PubsubIO transforms. */
+  private List<PTransformOverride> getPortableOverrides() {
+    return ImmutableList.of(
+        PTransformOverride.of(
+            PTransformMatchers.classEqualTo(PubsubUnboundedSource.class),
+            new DataflowReadFromPubsubSourceForRunnerV2OverrideFactory()),
+        PTransformOverride.of(

Review comment:
       We should also check `hasExperiment(options, "enable_custom_pubsub_sink")`




-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   It seems like it does fail in a reasonable about of time on master so I am worried that we don't have enough signal for this PR.


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   Looks like this doesn't fix it: 
   
   ```
   17:58:21 > Task :runners:google-cloud-dataflow-java:googleCloudPlatformRunnerV2IntegrationTest
   18:40:01 
   18:40:01 org.apache.beam.sdk.io.gcp.pubsub.PubsubReadIT > testReadPubsubMessageId FAILED
   18:40:01     java.lang.AssertionError at PubsubReadIT.java:88
   18:45:09 
   18:45:09 org.apache.beam.sdk.io.gcp.pubsub.PubsubReadIT > testReadPublicData FAILED
   18:45:09     java.lang.AssertionError at PubsubReadIT.java:60
   19:37:34 
   19:37:34 org.apache.beam.sdk.io.gcp.bigquery.BigQueryToTableIT > testNewTypesQueryWithoutReshuffle FAILED
   19:37:34     java.lang.RuntimeException at BigQueryToTableIT.java:116
   19:45:44 Build timed out (after 240 minutes). Marking the build as aborted.
   ```
   
   The last successful build took 3 hours. I have not investigated whether there are new tests added to this job.


-- 
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] boyuanzz commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   > We move this to apply to the portable proto because that is what is sent to runner v2. So without applying this, runner v2 does not use the native Pubsub, yes?
   
   Correct. we might have situation like even with dataflow, some customers doesn't want to use Dataflow native PubSub.


-- 
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] boyuanzz commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   https://ci-beam.apache.org/job/beam_PostCommit_Java/7360/ is broken from head because of HealthCareIO quota 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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   Run Java Examples on Dataflow Runner V2


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   Since #14299 is merged, I will rebase and we will be sure to have up to date test signal. But you can also view the link above if you are interested.


-- 
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] kennknowles commented on pull request #14297: [BEAM-12021] Add Pubsub overrides before portable proto translation in Dataflow

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


   Run Java Examples on Dataflow Runner V2


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