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/09/19 19:05:06 UTC

[GitHub] [beam] chamikaramj opened a new pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

chamikaramj opened a new pull request #12880:
URL: https://github.com/apache/beam/pull/12880


   These have to be done before creating the pipeline proto for Dataflow to be able to correctly generate job requests from portable protos.
   
   ------------------------
   
   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 | 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_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/i
 con)](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.apache.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](htt
 ps://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_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![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_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_P
 ostCommit_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_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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12880      +/-   ##
   ==========================================
   - Coverage   82.34%   82.33%   -0.02%     
   ==========================================
     Files         452      452              
     Lines       54016    54019       +3     
   ==========================================
   - Hits        44481    44474       -7     
   - Misses       9535     9545      +10     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.27% <100.00%> (+0.09%)` | :arrow_up: |
   | [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/localfilesystem.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vbG9jYWxmaWxlc3lzdGVtLnB5) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
   | [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.80% <0.00%> (-0.54%)` | :arrow_down: |
   | [...nners/portability/fn\_api\_runner/worker\_handlers.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3dvcmtlcl9oYW5kbGVycy5weQ==) | `80.57% <0.00%> (-0.18%)` | :arrow_down: |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [067cba8...23e6c0d](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] lukecwik commented on a change in pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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



##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -430,6 +430,15 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
                 components.coders[windowing_strategy.window_coder_id].spec.urn,
                 windowing_strategy.window_fn.urn))
 
+  def _adjust_types_for_dataflow(self, pipeline):
+    # Dataflow runner requires a KV type for GBK inputs, hence we enforce that
+    # here.
+    pipeline.visit(self.group_by_key_input_visitor())
+
+    # Dataflow runner requires output type of the Flatten to be the same as the

Review comment:
       @CraigChambersG I was under the impression that this wasn't true and that Dataflow v2 handled this.

##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -430,6 +430,11 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
                 components.coders[windowing_strategy.window_coder_id].spec.urn,
                 windowing_strategy.window_fn.urn))
 
+  def _adjust_types_for_dataflow(self, pipeline):

Review comment:
       _adjust_pipeline_for_dataflow_v2?

##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -488,12 +497,15 @@ def run_pipeline(self, pipeline, options):
       # Cross language transform require using a pipeline object constructed
       # from the full pipeline proto to make sure that expanded version of
       # external transforms are reflected in the Pipeline job graph.
+      # TODO(chamikara): remove following pipeline and pipeline proto recreation
+      # after portable job submission path is fully in place.
       from apache_beam import Pipeline
       pipeline = Pipeline.from_runner_api(
           self.proto_pipeline,
           pipeline.runner,
           options,
           allow_proto_holders=True)
+      self._adjust_types_for_dataflow(pipeline)

Review comment:
       Why do the type encodings not survive the round trip (pipeline -> proto -> pipeline)?




----------------------------------------------------------------
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 #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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



##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -488,12 +497,15 @@ def run_pipeline(self, pipeline, options):
       # Cross language transform require using a pipeline object constructed
       # from the full pipeline proto to make sure that expanded version of
       # external transforms are reflected in the Pipeline job graph.
+      # TODO(chamikara): remove following pipeline and pipeline proto recreation
+      # after portable job submission path is fully in place.
       from apache_beam import Pipeline
       pipeline = Pipeline.from_runner_api(
           self.proto_pipeline,
           pipeline.runner,
           options,
           allow_proto_holders=True)
+      self._adjust_types_for_dataflow(pipeline)

Review comment:
       Verified that types are preserved in the pipeline->proto->pipeline roundtrip and removed this.

##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -430,6 +430,11 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
                 components.coders[windowing_strategy.window_coder_id].spec.urn,
                 windowing_strategy.window_fn.urn))
 
+  def _adjust_types_for_dataflow(self, pipeline):

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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12880      +/-   ##
   ==========================================
   - Coverage   82.34%   82.33%   -0.02%     
   ==========================================
     Files         452      452              
     Lines       54016    54019       +3     
   ==========================================
   - Hits        44481    44474       -7     
   - Misses       9535     9545      +10     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.27% <100.00%> (+0.09%)` | :arrow_up: |
   | [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/localfilesystem.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vbG9jYWxmaWxlc3lzdGVtLnB5) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
   | [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.80% <0.00%> (-0.54%)` | :arrow_down: |
   | [...nners/portability/fn\_api\_runner/worker\_handlers.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3dvcmtlcl9oYW5kbGVycy5weQ==) | `80.57% <0.00%> (-0.18%)` | :arrow_down: |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [067cba8...23e6c0d](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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 merged pull request #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
chamikaramj merged pull request #12880:
URL: https://github.com/apache/beam/pull/12880


   


----------------------------------------------------------------
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 merged pull request #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
chamikaramj merged pull request #12880:
URL: https://github.com/apache/beam/pull/12880


   


----------------------------------------------------------------
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 #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

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


   Run Python_PVR_Flink 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 a change in pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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



##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -488,12 +497,15 @@ def run_pipeline(self, pipeline, options):
       # Cross language transform require using a pipeline object constructed
       # from the full pipeline proto to make sure that expanded version of
       # external transforms are reflected in the Pipeline job graph.
+      # TODO(chamikara): remove following pipeline and pipeline proto recreation
+      # after portable job submission path is fully in place.
       from apache_beam import Pipeline
       pipeline = Pipeline.from_runner_api(
           self.proto_pipeline,
           pipeline.runner,
           options,
           allow_proto_holders=True)
+      self._adjust_types_for_dataflow(pipeline)

Review comment:
       Verified that types are preserved in the pipeline->proto->pipeline roundtrip and removed this.

##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -430,6 +430,11 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
                 components.coders[windowing_strategy.window_coder_id].spec.urn,
                 windowing_strategy.window_fn.urn))
 
+  def _adjust_types_for_dataflow(self, pipeline):

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] chamikaramj commented on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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


   I see. I just moved Flatten update out of caution. I was not hitting this in any tests.
   
   GBK update seems to be needed though. I was hitting this in test_gbk_side_input (where we were failing while try to determine the coder for native shuffle reader). 


----------------------------------------------------------------
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 #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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


   Moved the Flatten change back and added a comment about Runner v2. PTAL.


----------------------------------------------------------------
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] CraigChambersG commented on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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


   Yes, that should be the case, on v2.  Dataflow v2 can handle
   heterogeneously typed Flattens.
   
   On Mon, Sep 21, 2020 at 9:10 AM Lukasz Cwik <no...@github.com>
   wrote:
   
   > *@lukecwik* commented on this pull request.
   > ------------------------------
   >
   > In sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
   > <https://github.com/apache/beam/pull/12880#discussion_r492181015>:
   >
   > > @@ -430,6 +430,15 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
   >                  components.coders[windowing_strategy.window_coder_id].spec.urn,
   >                  windowing_strategy.window_fn.urn))
   >
   > +  def _adjust_types_for_dataflow(self, pipeline):
   > +    # Dataflow runner requires a KV type for GBK inputs, hence we enforce that
   > +    # here.
   > +    pipeline.visit(self.group_by_key_input_visitor())
   > +
   > +    # Dataflow runner requires output type of the Flatten to be the same as the
   >
   > @CraigChambersG <https://github.com/CraigChambersG> I was under the
   > impression that this wasn't true and that Dataflow v2 handled this.
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/beam/pull/12880#pullrequestreview-492771941>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AKXWJXCTMD5YB2IIFZ5PAHLSG53HVANCNFSM4RTFTKVA>
   > .
   >
   


----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/1b266601c39f243f48dfd085b565b984f936d02c?el=desc) will **decrease** coverage by `0.00%`.
   > The diff coverage is `92.59%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12880      +/-   ##
   ==========================================
   - Coverage   82.32%   82.32%   -0.01%     
   ==========================================
     Files         452      453       +1     
     Lines       54016    54042      +26     
   ==========================================
   + Hits        44471    44492      +21     
   - Misses       9545     9550       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...python/apache\_beam/examples/wordcount\_dataframe.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvd29yZGNvdW50X2RhdGFmcmFtZS5weQ==) | `91.66% <91.66%> (ø)` | |
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.24% <100.00%> (+0.06%)` | :arrow_up: |
   | [sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=) | `88.75% <0.00%> (-0.45%)` | :arrow_down: |
   | [sdks/python/apache\_beam/transforms/combiners.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb21iaW5lcnMucHk=) | `91.97% <0.00%> (-0.19%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5) | `84.33% <0.00%> (+0.28%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [096f695...83a4d41](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] lukecwik commented on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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


   Yeah, the GBK one makes sense since it needs to say its a pair coder.


----------------------------------------------------------------
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] CraigChambersG commented on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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


   Yes, that should be the case, on v2.  Dataflow v2 can handle
   heterogeneously typed Flattens.
   
   On Mon, Sep 21, 2020 at 9:10 AM Lukasz Cwik <no...@github.com>
   wrote:
   
   > *@lukecwik* commented on this pull request.
   > ------------------------------
   >
   > In sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
   > <https://github.com/apache/beam/pull/12880#discussion_r492181015>:
   >
   > > @@ -430,6 +430,15 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
   >                  components.coders[windowing_strategy.window_coder_id].spec.urn,
   >                  windowing_strategy.window_fn.urn))
   >
   > +  def _adjust_types_for_dataflow(self, pipeline):
   > +    # Dataflow runner requires a KV type for GBK inputs, hence we enforce that
   > +    # here.
   > +    pipeline.visit(self.group_by_key_input_visitor())
   > +
   > +    # Dataflow runner requires output type of the Flatten to be the same as the
   >
   > @CraigChambersG <https://github.com/CraigChambersG> I was under the
   > impression that this wasn't true and that Dataflow v2 handled this.
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/beam/pull/12880#pullrequestreview-492771941>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AKXWJXCTMD5YB2IIFZ5PAHLSG53HVANCNFSM4RTFTKVA>
   > .
   >
   


----------------------------------------------------------------
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 #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

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






----------------------------------------------------------------
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 #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

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


   Run Python_PVR_Flink 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] lukecwik commented on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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


   Yeah, the GBK one makes sense since it needs to say its a pair coder.


----------------------------------------------------------------
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] lukecwik commented on a change in pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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



##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -430,6 +430,15 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
                 components.coders[windowing_strategy.window_coder_id].spec.urn,
                 windowing_strategy.window_fn.urn))
 
+  def _adjust_types_for_dataflow(self, pipeline):
+    # Dataflow runner requires a KV type for GBK inputs, hence we enforce that
+    # here.
+    pipeline.visit(self.group_by_key_input_visitor())
+
+    # Dataflow runner requires output type of the Flatten to be the same as the

Review comment:
       @CraigChambersG I was under the impression that this wasn't true and that Dataflow v2 handled 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] lukecwik commented on a change in pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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



##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -430,6 +430,11 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
                 components.coders[windowing_strategy.window_coder_id].spec.urn,
                 windowing_strategy.window_fn.urn))
 
+  def _adjust_types_for_dataflow(self, pipeline):

Review comment:
       _adjust_pipeline_for_dataflow_v2?

##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -488,12 +497,15 @@ def run_pipeline(self, pipeline, options):
       # Cross language transform require using a pipeline object constructed
       # from the full pipeline proto to make sure that expanded version of
       # external transforms are reflected in the Pipeline job graph.
+      # TODO(chamikara): remove following pipeline and pipeline proto recreation
+      # after portable job submission path is fully in place.
       from apache_beam import Pipeline
       pipeline = Pipeline.from_runner_api(
           self.proto_pipeline,
           pipeline.runner,
           options,
           allow_proto_holders=True)
+      self._adjust_types_for_dataflow(pipeline)

Review comment:
       Why do the type encodings not survive the round trip (pipeline -> proto -> pipeline)?




----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/1b266601c39f243f48dfd085b565b984f936d02c?el=desc) will **decrease** coverage by `0.00%`.
   > The diff coverage is `92.59%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12880      +/-   ##
   ==========================================
   - Coverage   82.32%   82.32%   -0.01%     
   ==========================================
     Files         452      453       +1     
     Lines       54016    54042      +26     
   ==========================================
   + Hits        44471    44492      +21     
   - Misses       9545     9550       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...python/apache\_beam/examples/wordcount\_dataframe.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvd29yZGNvdW50X2RhdGFmcmFtZS5weQ==) | `91.66% <91.66%> (ø)` | |
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.24% <100.00%> (+0.06%)` | :arrow_up: |
   | [sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=) | `88.75% <0.00%> (-0.45%)` | :arrow_down: |
   | [sdks/python/apache\_beam/transforms/combiners.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb21iaW5lcnMucHk=) | `91.97% <0.00%> (-0.19%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5) | `84.33% <0.00%> (+0.28%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [096f695...83a4d41](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] lukecwik commented on a change in pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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



##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -430,6 +430,11 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
                 components.coders[windowing_strategy.window_coder_id].spec.urn,
                 windowing_strategy.window_fn.urn))
 
+  def _adjust_types_for_dataflow(self, pipeline):

Review comment:
       _adjust_pipeline_for_dataflow_v2?

##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -488,12 +497,15 @@ def run_pipeline(self, pipeline, options):
       # Cross language transform require using a pipeline object constructed
       # from the full pipeline proto to make sure that expanded version of
       # external transforms are reflected in the Pipeline job graph.
+      # TODO(chamikara): remove following pipeline and pipeline proto recreation
+      # after portable job submission path is fully in place.
       from apache_beam import Pipeline
       pipeline = Pipeline.from_runner_api(
           self.proto_pipeline,
           pipeline.runner,
           options,
           allow_proto_holders=True)
+      self._adjust_types_for_dataflow(pipeline)

Review comment:
       Why do the type encodings not survive the round trip (pipeline -> proto -> pipeline)?




----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/1b266601c39f243f48dfd085b565b984f936d02c?el=desc) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #12880   +/-   ##
   =======================================
     Coverage   82.32%   82.32%           
   =======================================
     Files         452      452           
     Lines       54016    54018    +2     
   =======================================
   + Hits        44471    44473    +2     
     Misses       9545     9545           
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.24% <100.00%> (+0.06%)` | :arrow_up: |
   | [sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=) | `88.75% <0.00%> (-0.45%)` | :arrow_down: |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.18% <0.00%> (-0.27%)` | :arrow_down: |
   | [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.80% <0.00%> (-0.18%)` | :arrow_down: |
   | [...nners/portability/fn\_api\_runner/worker\_handlers.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3dvcmtlcl9oYW5kbGVycy5weQ==) | `80.75% <0.00%> (+0.17%)` | :arrow_up: |
   | [sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5) | `84.33% <0.00%> (+0.28%)` | :arrow_up: |
   | [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.90% <0.00%> (+1.22%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [096f695...83a4d41](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/1b266601c39f243f48dfd085b565b984f936d02c?el=desc) will **decrease** coverage by `0.00%`.
   > The diff coverage is `92.59%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12880      +/-   ##
   ==========================================
   - Coverage   82.32%   82.32%   -0.01%     
   ==========================================
     Files         452      453       +1     
     Lines       54016    54042      +26     
   ==========================================
   + Hits        44471    44492      +21     
   - Misses       9545     9550       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...python/apache\_beam/examples/wordcount\_dataframe.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZXhhbXBsZXMvd29yZGNvdW50X2RhdGFmcmFtZS5weQ==) | `91.66% <91.66%> (ø)` | |
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.24% <100.00%> (+0.06%)` | :arrow_up: |
   | [sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=) | `88.75% <0.00%> (-0.45%)` | :arrow_down: |
   | [sdks/python/apache\_beam/transforms/combiners.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb21iaW5lcnMucHk=) | `91.97% <0.00%> (-0.19%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5) | `84.33% <0.00%> (+0.28%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [096f695...83a4d41](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831






----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12880      +/-   ##
   ==========================================
   - Coverage   82.34%   82.32%   -0.02%     
   ==========================================
     Files         452      452              
     Lines       54016    54018       +2     
   ==========================================
   - Hits        44481    44473       -8     
   - Misses       9535     9545      +10     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.24% <100.00%> (+0.06%)` | :arrow_up: |
   | [sdks/python/apache\_beam/io/localfilesystem.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vbG9jYWxmaWxlc3lzdGVtLnB5) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
   | [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.80% <0.00%> (-0.54%)` | :arrow_down: |
   | [sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=) | `88.75% <0.00%> (-0.45%)` | :arrow_down: |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.18% <0.00%> (-0.40%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [067cba8...38a941d](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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 merged pull request #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
chamikaramj merged pull request #12880:
URL: https://github.com/apache/beam/pull/12880


   


----------------------------------------------------------------
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 #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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



##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -488,12 +497,15 @@ def run_pipeline(self, pipeline, options):
       # Cross language transform require using a pipeline object constructed
       # from the full pipeline proto to make sure that expanded version of
       # external transforms are reflected in the Pipeline job graph.
+      # TODO(chamikara): remove following pipeline and pipeline proto recreation
+      # after portable job submission path is fully in place.
       from apache_beam import Pipeline
       pipeline = Pipeline.from_runner_api(
           self.proto_pipeline,
           pipeline.runner,
           options,
           allow_proto_holders=True)
+      self._adjust_types_for_dataflow(pipeline)

Review comment:
       Verified that types are preserved in the pipeline->proto->pipeline roundtrip and removed this.

##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
##########
@@ -430,6 +430,11 @@ def _check_for_unsupported_fnapi_features(self, pipeline_proto):
                 components.coders[windowing_strategy.window_coder_id].spec.urn,
                 windowing_strategy.window_fn.urn))
 
+  def _adjust_types_for_dataflow(self, pipeline):

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] codecov[bot] commented on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12880      +/-   ##
   ==========================================
   - Coverage   82.34%   82.33%   -0.02%     
   ==========================================
     Files         452      452              
     Lines       54016    54019       +3     
   ==========================================
   - Hits        44481    44474       -7     
   - Misses       9535     9545      +10     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.27% <100.00%> (+0.09%)` | :arrow_up: |
   | [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/localfilesystem.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vbG9jYWxmaWxlc3lzdGVtLnB5) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
   | [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.80% <0.00%> (-0.54%)` | :arrow_down: |
   | [...nners/portability/fn\_api\_runner/worker\_handlers.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3dvcmtlcl9oYW5kbGVycy5weQ==) | `80.57% <0.00%> (-0.18%)` | :arrow_down: |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [067cba8...23e6c0d](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831






----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/1b266601c39f243f48dfd085b565b984f936d02c?el=desc) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #12880   +/-   ##
   =======================================
     Coverage   82.32%   82.32%           
   =======================================
     Files         452      452           
     Lines       54016    54018    +2     
   =======================================
   + Hits        44471    44473    +2     
     Misses       9545     9545           
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.24% <100.00%> (+0.06%)` | :arrow_up: |
   | [sdks/python/apache\_beam/runners/common.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9jb21tb24ucHk=) | `88.75% <0.00%> (-0.45%)` | :arrow_down: |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.18% <0.00%> (-0.27%)` | :arrow_down: |
   | [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.80% <0.00%> (-0.18%)` | :arrow_down: |
   | [...nners/portability/fn\_api\_runner/worker\_handlers.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3dvcmtlcl9oYW5kbGVycy5weQ==) | `80.75% <0.00%> (+0.17%)` | :arrow_up: |
   | [sdks/python/apache\_beam/io/iobase.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vaW9iYXNlLnB5) | `84.33% <0.00%> (+0.28%)` | :arrow_up: |
   | [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.90% <0.00%> (+1.22%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [096f695...83a4d41](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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 #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

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






----------------------------------------------------------------
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] codecov[bot] edited a comment on pull request #12880: [BEAM-10933] Adjust GBK and Flatten types before creating the pipeline proto

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12880:
URL: https://github.com/apache/beam/pull/12880#issuecomment-695346831


   # [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=h1) Report
   > Merging [#12880](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/beam/pull/12880/graphs/tree.svg?width=650&height=150&src=pr&token=qcbbAh8Fj1)](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12880      +/-   ##
   ==========================================
   - Coverage   82.34%   82.33%   -0.02%     
   ==========================================
     Files         452      452              
     Lines       54016    54019       +3     
   ==========================================
   - Hits        44481    44474       -7     
   - Misses       9535     9545      +10     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...on/apache\_beam/runners/dataflow/dataflow\_runner.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19ydW5uZXIucHk=) | `77.27% <100.00%> (+0.09%)` | :arrow_up: |
   | [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.68% <0.00%> (-1.23%)` | :arrow_down: |
   | [sdks/python/apache\_beam/io/localfilesystem.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vbG9jYWxmaWxlc3lzdGVtLnB5) | `90.90% <0.00%> (-0.76%)` | :arrow_down: |
   | [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.80% <0.00%> (-0.54%)` | :arrow_down: |
   | [...nners/portability/fn\_api\_runner/worker\_handlers.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL3dvcmtlcl9oYW5kbGVycy5weQ==) | `80.57% <0.00%> (-0.18%)` | :arrow_down: |
   | [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12880/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.45% <0.00%> (-0.14%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=footer). Last update [067cba8...38a941d](https://codecov.io/gh/apache/beam/pull/12880?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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 #12880: [BEAM-10933] Adjust GBK type before creating the pipeline proto

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


   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