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 23:00:22 UTC
[GitHub] [beam] chadrik opened a new pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
chadrik opened a new pull request #12881:
URL: https://github.com/apache/beam/pull/12881
Ok, I'm back on the horse, but who knows for how long.
I reviewed the last PR that stalled out and the recommendation there was to focus on a single module or package. Let me know if the scope is too large on this one.
------------------------
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 | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](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 | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/)
Portable | --- | [](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)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](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] chadrik edited a comment on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698675941
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491596716
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -1070,23 +1078,24 @@ def delayed_bundle_application(self,
return beam_fn_api_pb2.DelayedBundleApplication(
requested_time_delay=proto_deferred_watermark,
application=self.construct_bundle_application(
- op, current_watermark, element_and_restriction))
+ op.input_info, current_watermark, element_and_restriction))
def bundle_application(self,
op, # type: operations.DoOperation
primary # type: SplitResultPrimary
):
# type: (...) -> beam_fn_api_pb2.BundleApplication
- return self.construct_bundle_application(op, None, primary.primary_value)
+ assert op.input_info is not None
+ return self.construct_bundle_application(
+ op.input_info, None, primary.primary_value)
def construct_bundle_application(self,
- op, # type: operations.DoOperation
+ op_input_info, # type: operations.OpInputInfo
Review comment:
Instead of passing around a `DoOperation` I switched this to a `OpInputInfo` because the latter is all that we need, and doing so lets us avoid having to assert that it is not `None` everywhere that it's used.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robertwb commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698656899
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r492493751
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -81,7 +85,11 @@
class ClosableOutputStream(OutputStream):
"""A Outputstream for use with CoderImpls that has a close() method."""
- def __init__(self, close_callback=None):
+ def __init__(
+ self,
+ close_callback=None # type: Optional[Optional[Callable[[bytes], None]]]
Review comment:
mistake. fixed.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491598497
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -947,11 +955,11 @@ def process_bundle(self, instruction_id):
# (transform_id, timer_family_id).
data_channels = collections.defaultdict(
list
- ) # type: DefaultDict[data_plane.GrpcClientDataChannel, List[str]]
+ ) # type: DefaultDict[data_plane.GrpcClientDataChannel, List[Union[str, Tuple[str, str]]]]
Review comment:
This can in fact contain items which are `Tuple[str, str]`. see:
```python
data_channels[self.timer_data_channel].extend(
list(self.timers_info.keys()))
```
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?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 `89.16%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.32% -0.02%
==========================================
Files 452 454 +2
Lines 54016 54109 +93
==========================================
+ Hits 44481 44547 +66
- Misses 9535 9562 +27
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.12% <0.00%> (-0.14%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <85.71%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.07% <89.47%> (-0.51%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [15 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...f252b9c](https://codecov.io/gh/apache/beam/pull/12881?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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/52698a42084100b70cebeb867925859b1c583ec0?el=desc) will **decrease** coverage by `0.17%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.51% 82.33% -0.18%
==========================================
Files 453 456 +3
Lines 54610 54657 +47
==========================================
- Hits 45061 45004 -57
- Misses 9549 9653 +104
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.98% <84.84%> (-1.17%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <85.71%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.11%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [28 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [969b0c6...64a14e8](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491590441
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -920,7 +928,7 @@ def reset(self):
def process_bundle(self, instruction_id):
# type: (str) -> Tuple[List[beam_fn_api_pb2.DelayedBundleApplication], bool]
- expected_inputs = []
+ expected_input_ops = []
Review comment:
This needed to be renamed because the variable was reused below for a different type
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491602968
##########
File path: sdks/python/apache_beam/utils/sentinel.py
##########
@@ -0,0 +1,30 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# pytype: skip-file
+
+from __future__ import absolute_import
+
+import enum
+
+
+class Sentinel(enum.Enum):
+ """
+ A type-safe sentinel class
+ """
+
+ sentinel = object()
Review comment:
see this [stackoverflow post](https://stackoverflow.com/questions/57959664/handling-conditional-logic-sentinel-value-with-mypy) for an explanation of why I chose this design for a type-safe sentinel.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491600011
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -243,7 +274,7 @@ def output_stream(
instruction_id, # type: str
transform_id # type: str
):
- # type: (...) -> ClosableOutputStream
+ # type: (...) -> SizeBasedBufferingClosableOutputStream
Review comment:
`ClosableOutputStream` seems to not actually be used anywhere any more, because `ClosableOutputStream.create()` returns `SizeBasedBufferingClosableOutputStream`. `ClosableOutputStream` does not implement the necessary `maybe_flush` method that's required everywhere we were referring to `ClosableOutputStream`
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491599345
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -218,7 +249,7 @@ class DataChannel(with_metaclass(abc.ABCMeta, object)): # type: ignore[misc]
@abc.abstractmethod
def input_elements(self,
instruction_id, # type: str
- expected_inputs, # type: Collection[str]
+ expected_inputs, # type: Sized
Review comment:
we just call `len(expected_inputs)`, so we just need to know this implements `__len__`
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491595091
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -705,10 +713,11 @@ def __init__(self,
self._key_coder = key_coder
self._window_coder = window_coder
# A mapping of {timer_family_id: TimerInfo}
- self._timers_info = {}
- self._all_states = {} # type: Dict[tuple, userstate.RuntimeState]
+ self._timers_info = {} # type: Dict[str, TimerInfo]
+ self._all_states = {} # type: Dict[tuple, FnApiUserRuntimeStateTypes]
Review comment:
`userstate.RuntimeState` does not have the necessary `commit()` method, and there is no common base class for these 4 types that has all of the necessary methods. When dealing with a reasonably limited number of possibilities with unclear parentage, a `Union` is a good solution.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r494687447
##########
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##########
@@ -622,10 +659,12 @@ def process_bundle_progress_metadata_request(self,
request, # type: beam_fn_api_pb2.ProcessBundleProgressMetadataRequest
instruction_id # type: str
):
+ # type: (...) -> beam_fn_api_pb2.InstructionResponse
return beam_fn_api_pb2.InstructionResponse(
instruction_id=instruction_id,
- process_bundle_progress=beam_fn_api_pb2.
+ process_bundle_progress_metadata=beam_fn_api_pb2.
ProcessBundleProgressMetadataResponse(
+ # FIXME: incompatible type "List[MonitoringInfo]"; expected "Optional[Mapping[str, Any]]"
monitoring_info=SHORT_ID_CACHE.getInfos(
request.monitoring_info_id)))
Review comment:
Note this change and the FIXME. I'm not sure what to do about this.
I got complaints that `InstructionResponse(process_bundle_progress=...)` was receiving the wrong type. I'm not sure whether this is the right resolution.
Also, `ProcessBundleProgressMetadataResponse(monitoring_info=...)`is also receiving the wrong type. It's supposed to be a mapping not a list. Should I change `SHORT_ID_CACHE.getInfos` to return a dictionary?
Do you know if this code is being hit? If so, any theories on how it's working?
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -966,6 +976,7 @@ def process_bundle(self, instruction_id):
output_stream = self.timer_data_channel.output_timer_stream(
instruction_id, transform_id, timer_family_id)
timer_info.output_stream = output_stream
+ # FIXME: how do we know that self.ops[transform_id] is a DoOperation?
self.ops[transform_id].add_timer_info(timer_family_id, timer_info)
Review comment:
`BundleProcessor.ops` is `OrderedDict[str, operations.Operation]`. Should it be `OrderedDict[str, operations.DoOperation]`? Or do the transform ids in `BundleProcessor.timers_info` all point to `DoOperations`?
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491735659
##########
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##########
@@ -174,11 +191,14 @@ def __init__(self,
self._state_handler_factory = GrpcStateHandlerFactory(
self._state_cache, credentials)
self._profiler_factory = profiler_factory
- self._fns = KeyedDefaultDict(
- lambda id: self._control_stub.GetProcessBundleDescriptor(
- beam_fn_api_pb2.GetProcessBundleDescriptorRequest(
- process_bundle_descriptor_id=id))
- ) # type: Mapping[str, beam_fn_api_pb2.ProcessBundleDescriptor]
+
+ def default_factory(id):
+ # type: (str) -> beam_fn_api_pb2.ProcessBundleDescriptor
+ return self._control_stub.GetProcessBundleDescriptor(
+ beam_fn_api_pb2.GetProcessBundleDescriptorRequest(
+ process_bundle_descriptor_id=id))
+
+ self._fns = KeyedDefaultDict(default_factory)
Review comment:
changing this to a function lets us add annotations which silences a mypy error. mypy can now detect the key/value types of `KeyedDefaultDict`
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.31% -0.04%
==========================================
Files 452 456 +4
Lines 54016 54657 +641
==========================================
+ Hits 44481 44991 +510
- Misses 9535 9666 +131
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (-0.12%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.32% <85.71%> (-1.59%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [58 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...e1483c9](https://codecov.io/gh/apache/beam/pull/12881?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] robertwb merged pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
robertwb merged pull request #12881:
URL: https://github.com/apache/beam/pull/12881
----------------------------------------------------------------
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] chadrik commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-702430284
We're getting merge conflicts. I'm thinking of just ignoring the remaining errors, so that we can just get this merged. I can add a FIXME referencing BEAM-7746. Thoughts?
The one I'm most concerned about is `ProcessBundleProgressMetadataResponse`, which seems like an actual bug. @lukecwik any thoughts on that one?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491735746
##########
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##########
@@ -464,16 +496,17 @@ class SdkWorker(object):
def __init__(self,
bundle_processor_cache, # type: BundleProcessorCache
- state_cache_metrics_fn=list,
+ state_cache_metrics_fn=list, # type: Callable[[], Iterable[metrics_pb2.MonitoringInfo]]
profiler_factory=None, # type: Optional[Callable[..., Profile]]
- log_lull_timeout_ns=None,
+ log_lull_timeout_ns=None, # type: Optional[int]
):
+ # type: (...) -> None
self.bundle_processor_cache = bundle_processor_cache
self.state_cache_metrics_fn = state_cache_metrics_fn
self.profiler_factory = profiler_factory
self.log_lull_timeout_ns = (
log_lull_timeout_ns or DEFAULT_LOG_LULL_TIMEOUT_NS)
- self._last_full_thread_dump_secs = 0
+ self._last_full_thread_dump_secs = 0.0
Review comment:
this attribute eventually gets set to the value of `time.time()` which is a `float`.
----------------------------------------------------------------
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] chadrik edited a comment on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698675941
> > 23:36:24 apache_beam/runners/worker/data_plane.py:714: error: Incompatible return value type (got "Optional[GrpcClientDataChannel]", expected "GrpcClientDataChannel") [return-value]
>
> Sure this shouldn't return optional as well?
When I do that, it just causes errors elsewhere. I couldn't find a solution. I think there's some implicit guarantee going on here. I'd like to identify where it is and add an assert, a cast, and/or a comment, but I don't know the system well enough.
> > 23:36:24 apache_beam/runners/worker/operations.py:722: error: Argument 1 to "append" of "list" has incompatible type "Tuple[DoOperation, Iterable[SplitResultResidual]]"; expected "Tuple[DoOperation, SplitResultResidual]" [arg-type]
>
> self.dofn_runner.process does return an `Iterable[SplitResultResidual]`
yeah, the problem is that `self.execution_context.delayed_applications` is a list of `Tuple[..., SplitResultResidual]`.
and we're trying to append to that list a `Tuple[..., Iterable[SplitResultResidual]]`. Should this be a list of `Union`, or is something wrong here?
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:241: error: Signature of "try_split" incompatible with supertype "Operation" [override]
>
> The supertype returns Optional[Any], which should be compatible with this.
The number of arguments is different.
>
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:978: error: "Operation" has no attribute "add_timer_info" [attr-defined]
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:990: error: "Operation" has no attribute "process_timer" [attr-defined]
>
> These will always be DoOperations.
Does `create_execution_tree` return `OrderedDict[str, operations.DoOperation]`?
>
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:1568: error: Argument 5 to "StateBackedSideInputMap" has incompatible type "Coder"; expected "WindowedValueCoder" [arg-type]
>
> I think this will always hold, we just don't know it due to having deserialized it with the generic mechanisms.
I changed `BeamTransformFactory.get_input_coders` to return `Dict[str, coders.WindowedValueCoder]`. Does that seem right?
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/52698a42084100b70cebeb867925859b1c583ec0?el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `91.06%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.51% 82.50% -0.02%
==========================================
Files 453 455 +2
Lines 54610 54786 +176
==========================================
+ Hits 45061 45199 +138
- Misses 9549 9587 +38
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/profiler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvcHJvZmlsZXIucHk=) | `86.45% <80.00%> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.47% <86.11%> (-0.68%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <86.66%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.47% <90.47%> (+0.02%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/doctests.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2RvY3Rlc3RzLnB5) | `96.75% <96.22%> (-0.45%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/filebasedsource.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZmlsZWJhc2Vkc291cmNlLnB5) | `98.77% <100.00%> (+0.02%)` | :arrow_up: |
| ... and [22 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [969b0c6...f6dd36b](https://codecov.io/gh/apache/beam/pull/12881?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] robertwb commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-703802592
Yeah, PTransformTest.test_flatten_no_pcollections is flaky. Lint, however, isn't.
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.31% -0.04%
==========================================
Files 452 456 +4
Lines 54016 54657 +641
==========================================
+ Hits 44481 44991 +510
- Misses 9535 9666 +131
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (-0.12%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.32% <85.71%> (-1.59%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [58 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...e1483c9](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491592170
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -303,9 +334,10 @@ def inverse(self):
def input_elements(self,
instruction_id, # type: str
- unused_expected_inputs=None, # type: Collection[str]
+ unused_expected_inputs, # type: Any
Review comment:
It's more accurate to say `Any` here, since the method doesn't care. The argument can't be `Optional` or the method it would be incompatible with its super type.
I checked all uses of `input_elements` and I did not see any case where it was called with only one arg.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491593835
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -1125,15 +1134,15 @@ def shutdown(self):
class ExecutionContext(object):
def __init__(self):
self.delayed_applications = [
- ] # type: List[Tuple[operations.DoOperation, common.SplitResultType]]
+ ] # type: List[Tuple[operations.DoOperation, common.SplitResultResidual]]
Review comment:
this was just wrong. there is no `common.SplitResultType`
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491597504
##########
File path: sdks/python/apache_beam/utils/urns.py
##########
@@ -177,7 +178,7 @@ def to_runner_api(self, context):
@classmethod
def from_runner_api(cls, fn_proto, context):
- # type: (beam_runner_api_pb2.FunctionSpec, PipelineContext) -> Any
+ # type: (Type[RunnerApiFnT], beam_runner_api_pb2.FunctionSpec, PipelineContext) -> RunnerApiFnT
Review comment:
This change indicates that we should return an instance of the class that this is called from, though it provides no runtime guarantees of 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] chadrik commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698673308
> apache_beam/runners/worker/operations.py:722
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491592170
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -303,9 +334,10 @@ def inverse(self):
def input_elements(self,
instruction_id, # type: str
- unused_expected_inputs=None, # type: Collection[str]
+ unused_expected_inputs, # type: Any
Review comment:
This method was incompatible with its super type. If it is `None` it invalidates the Liskov substitution principle.
It's more accurate to say `Any` here, since the method doesn't care, and this keeps the method compatible with its super type.
I checked all uses of `input_elements` and I did not see any case where it was called with only one arg.
----------------------------------------------------------------
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] chadrik removed a comment on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik removed a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698673308
> apache_beam/runners/worker/operations.py:722
----------------------------------------------------------------
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] chadrik commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698675941
> > 23:36:24 apache_beam/runners/worker/data_plane.py:714: error: Incompatible return value type (got "Optional[GrpcClientDataChannel]", expected "GrpcClientDataChannel") [return-value]
>
> Sure this shouldn't return optional as well?
When I do that, it just causes errors elsewhere. I couldn't find a solution. I think there's some implicit guarantee going on here. I'd like to identify where it is and add an assert, a cast, and/or a comment, but I don't know the system well enough.
> > 23:36:24 apache_beam/runners/worker/operations.py:722: error: Argument 1 to "append" of "list" has incompatible type "Tuple[DoOperation, Iterable[SplitResultResidual]]"; expected "Tuple[DoOperation, SplitResultResidual]" [arg-type]
>
> self.dofn_runner.process does return an `Iterable[SplitResultResidual]`
yeah, the problem is that `self.execution_context.delayed_applications` is `List[Tuple[operations.DoOperation, common.SplitResultResidual]]`, and we're trying to append to it. Should this be `Union[Iterable[SplitResultResidual], SplitResultResidual]`, or is something wrong here?
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:241: error: Signature of "try_split" incompatible with supertype "Operation" [override]
>
> The supertype returns Optional[Any], which should be compatible with this.
The number of arguments is different.
>
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:978: error: "Operation" has no attribute "add_timer_info" [attr-defined]
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:990: error: "Operation" has no attribute "process_timer" [attr-defined]
>
> These will always be DoOperations.
Does `create_execution_tree` return `OrderedDict[str, operations.DoOperation]`?
>
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:1568: error: Argument 5 to "StateBackedSideInputMap" has incompatible type "Coder"; expected "WindowedValueCoder" [arg-type]
>
> I think this will always hold, we just don't know it due to having deserialized it with the generic mechanisms.
I changed `BeamTransformFactory.get_input_coders` to return `Dict[str, coders.WindowedValueCoder]`. Does that seem right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robertwb commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-702459845
Yes, let's fix `ProcessBundleProgressMetadataResponse`, drop some TODOs, enable what checks we can, and get this merged.
----------------------------------------------------------------
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] chadrik closed pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik closed pull request #12881:
URL: https://github.com/apache/beam/pull/12881
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robertwb commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698656899
> 23:36:24 apache_beam/runners/worker/data_plane.py:714: error: Incompatible return value type (got "Optional[GrpcClientDataChannel]", expected "GrpcClientDataChannel") [return-value]
Sure this shouldn't return optional as well?
> 23:36:24 apache_beam/runners/worker/operations.py:722: error: Argument 1 to "append" of "list" has incompatible type "Tuple[DoOperation, Iterable[SplitResultResidual]]"; expected "Tuple[DoOperation, SplitResultResidual]" [arg-type]
self.dofn_runner.process does return an `Iterable[SplitResultResidual]`
> 23:36:24 apache_beam/runners/worker/bundle_processor.py:241: error: Signature of "try_split" incompatible with supertype "Operation" [override]
The supertype returns Optional[Any], which should be compatible with this.
> 23:36:24 apache_beam/runners/worker/bundle_processor.py:978: error: "Operation" has no attribute "add_timer_info" [attr-defined]
> 23:36:24 apache_beam/runners/worker/bundle_processor.py:990: error: "Operation" has no attribute "process_timer" [attr-defined]
These will always be DoOperations.
>23:36:24 apache_beam/runners/worker/bundle_processor.py:1568: error: Argument 5 to "StateBackedSideInputMap" has incompatible type "Coder"; expected "WindowedValueCoder" [arg-type]
I think this will always hold, we just don't know it due to having deserialized it with the generic mechanisms.
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.31% -0.04%
==========================================
Files 452 456 +4
Lines 54016 54657 +641
==========================================
+ Hits 44481 44991 +510
- Misses 9535 9666 +131
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (-0.12%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.32% <85.71%> (-1.59%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [58 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...e1483c9](https://codecov.io/gh/apache/beam/pull/12881?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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.31% -0.04%
==========================================
Files 452 456 +4
Lines 54016 54657 +641
==========================================
+ Hits 44481 44991 +510
- Misses 9535 9666 +131
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (-0.12%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.32% <85.71%> (-1.59%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [58 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...e1483c9](https://codecov.io/gh/apache/beam/pull/12881?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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.33% -0.01%
==========================================
Files 452 456 +4
Lines 54016 54657 +641
==========================================
+ Hits 44481 45004 +523
- Misses 9535 9653 +118
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (-0.12%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.98% <84.84%> (-0.36%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <85.71%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [56 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...e1483c9](https://codecov.io/gh/apache/beam/pull/12881?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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?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 `89.16%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.32% -0.02%
==========================================
Files 452 454 +2
Lines 54016 54109 +93
==========================================
+ Hits 44481 44547 +66
- Misses 9535 9562 +27
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.12% <0.00%> (-0.14%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <85.71%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.07% <89.47%> (-0.51%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [15 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...f252b9c](https://codecov.io/gh/apache/beam/pull/12881?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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.31% -0.04%
==========================================
Files 452 456 +4
Lines 54016 54657 +641
==========================================
+ Hits 44481 44991 +510
- Misses 9535 9666 +131
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (-0.12%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.32% <85.71%> (-1.59%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [58 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...e1483c9](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-702501121
Done.
On Thu, Oct 1, 2020 at 5:10 PM Robert Bradshaw <no...@github.com>
wrote:
> Yes, let's fix ProcessBundleProgressMetadataResponse, drop some TODOs,
> enable what checks we can, and get this merged.
>
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/beam/pull/12881#issuecomment-702459845>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAPOEZJGVQAT2XSWV5Z3YTSIUK7NANCNFSM4RTI55LQ>
> .
>
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?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 `89.16%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.32% -0.02%
==========================================
Files 452 454 +2
Lines 54016 54109 +93
==========================================
+ Hits 44481 44547 +66
- Misses 9535 9562 +27
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.12% <0.00%> (-0.14%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <85.71%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.07% <89.47%> (-0.51%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [15 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...f252b9c](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491593456
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -1344,7 +1356,8 @@ def create_deprecated_read(
consumers # type: Dict[str, List[operations.Operation]]
):
# type: (...) -> operations.ReadOperation
- source = iobase.SourceBase.from_runner_api(parameter.source, factory.context)
+ source = iobase.BoundedSource.from_runner_api(
Review comment:
This shouldn't change the behavior of the code, it merely indicates to the type system that we expect `BoundedSource`, since `SourceBase` is not sufficient here (we need `default_output_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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491722757
##########
File path: sdks/python/apache_beam/runners/worker/statecache.py
##########
@@ -44,15 +48,17 @@ class Metrics(object):
PREFIX = "beam:metric:statecache:"
def __init__(self):
+ # type: () -> None
self._context = threading.local()
def initialize(self):
+ # type: () -> None
+
"""Needs to be called once per thread to initialize the local metrics cache.
"""
if hasattr(self._context, 'metrics'):
return # Already initialized
- self._context.metrics = collections.defaultdict(
- int) # type: DefaultDict[Hashable, int]
Review comment:
Removed this annotation because it's invalid: you can't add annotations to an attribute of another class (non-self attribute). I've noticed there's definitely a need for a typed version of `threading.local`.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r494687447
##########
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##########
@@ -622,10 +659,12 @@ def process_bundle_progress_metadata_request(self,
request, # type: beam_fn_api_pb2.ProcessBundleProgressMetadataRequest
instruction_id # type: str
):
+ # type: (...) -> beam_fn_api_pb2.InstructionResponse
return beam_fn_api_pb2.InstructionResponse(
instruction_id=instruction_id,
- process_bundle_progress=beam_fn_api_pb2.
+ process_bundle_progress_metadata=beam_fn_api_pb2.
ProcessBundleProgressMetadataResponse(
+ # FIXME: incompatible type "List[MonitoringInfo]"; expected "Optional[Mapping[str, Any]]"
monitoring_info=SHORT_ID_CACHE.getInfos(
request.monitoring_info_id)))
Review comment:
Note this change and the FIXME. I'm not sure what to do about this.
I got complaints that `InstructionResponse(process_bundle_progress=...)` was receiving the wrong type. I'm not sure whether this is the right resolution.
Also, `ProcessBundleProgressMetadataResponse(monitoring_info=...)`is also receiving the wrong type. It's supposed to be a mapping not a list. Should I change `SHORT_ID_CACHE.getInfos` to return a dictionary?
Do you know if this code is being hit? If so, any theories on how it's working?
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.33% -0.01%
==========================================
Files 452 456 +4
Lines 54016 54657 +641
==========================================
+ Hits 44481 45004 +523
- Misses 9535 9653 +118
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (-0.12%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.98% <84.84%> (-0.36%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <85.71%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [56 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...e1483c9](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik closed pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik closed pull request #12881:
URL: https://github.com/apache/beam/pull/12881
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r492496954
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -243,7 +274,7 @@ def output_stream(
instruction_id, # type: str
transform_id # type: str
):
- # type: (...) -> ClosableOutputStream
+ # type: (...) -> SizeBasedBufferingClosableOutputStream
Review comment:
fixed.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491722757
##########
File path: sdks/python/apache_beam/runners/worker/statecache.py
##########
@@ -44,15 +48,17 @@ class Metrics(object):
PREFIX = "beam:metric:statecache:"
def __init__(self):
+ # type: () -> None
self._context = threading.local()
def initialize(self):
+ # type: () -> None
+
"""Needs to be called once per thread to initialize the local metrics cache.
"""
if hasattr(self._context, 'metrics'):
return # Already initialized
- self._context.metrics = collections.defaultdict(
- int) # type: DefaultDict[Hashable, int]
Review comment:
Removed this annotation because it's invalid: you can't add annotations to an attribute of another class (non-self attribute). I've noticed there's definitely a need for a typed version of `threading.local`.
##########
File path: sdks/python/apache_beam/runners/worker/log_handler.py
##########
@@ -156,9 +174,11 @@ def _write_log_entries(self):
done = True
log_entries.pop()
if log_entries:
- yield beam_fn_api_pb2.LogEntry.List(log_entries=log_entries)
+ yield beam_fn_api_pb2.LogEntry.List(
+ log_entries=cast(List[beam_fn_api_pb2.LogEntry], log_entries))
Review comment:
we have to cast here because `log_entries` started out life as a `List[Union[..., Sentinel]]`, and just above this line we checked for the sentinel and popped it out, but mypy can't track that.
##########
File path: sdks/python/apache_beam/runners/worker/log_handler.py
##########
@@ -156,9 +174,11 @@ def _write_log_entries(self):
done = True
log_entries.pop()
if log_entries:
- yield beam_fn_api_pb2.LogEntry.List(log_entries=log_entries)
+ yield beam_fn_api_pb2.LogEntry.List(
+ log_entries=cast(List[beam_fn_api_pb2.LogEntry], log_entries))
Review comment:
we have to cast here because `log_entries` was initialized as a `List[Union[..., Sentinel]]`, and just above this line we checked for the sentinel and popped it out, but mypy can't track that.
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -303,9 +334,10 @@ def inverse(self):
def input_elements(self,
instruction_id, # type: str
- unused_expected_inputs=None, # type: Collection[str]
+ unused_expected_inputs, # type: Any
Review comment:
It's more accurate to say `Any` here, since the method doesn't care. The argument can't be `Optional` or the method it would be incompatible with its super type.
I checked all uses of `input_elements` and I did not see any case where it was called with only one arg.
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -1619,7 +1633,7 @@ def _create_pardo_operation(
consumers,
output_tags)
if pardo_proto and pardo_proto.restriction_coder_id:
- result.input_info = (
+ result.input_info = operations.OpInputInfo(
Review comment:
Making this a NamedTuple added a bit of sanity, and should be completely safe.
##########
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##########
@@ -174,11 +191,14 @@ def __init__(self,
self._state_handler_factory = GrpcStateHandlerFactory(
self._state_cache, credentials)
self._profiler_factory = profiler_factory
- self._fns = KeyedDefaultDict(
- lambda id: self._control_stub.GetProcessBundleDescriptor(
- beam_fn_api_pb2.GetProcessBundleDescriptorRequest(
- process_bundle_descriptor_id=id))
- ) # type: Mapping[str, beam_fn_api_pb2.ProcessBundleDescriptor]
+
+ def default_factory(id):
+ # type: (str) -> beam_fn_api_pb2.ProcessBundleDescriptor
+ return self._control_stub.GetProcessBundleDescriptor(
+ beam_fn_api_pb2.GetProcessBundleDescriptorRequest(
+ process_bundle_descriptor_id=id))
+
+ self._fns = KeyedDefaultDict(default_factory)
Review comment:
changing this to a function lets us add annotations which silences a mypy error. mypy can now detect the key/value types of `KeyedDefaultDict`
##########
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##########
@@ -464,16 +496,17 @@ class SdkWorker(object):
def __init__(self,
bundle_processor_cache, # type: BundleProcessorCache
- state_cache_metrics_fn=list,
+ state_cache_metrics_fn=list, # type: Callable[[], Iterable[metrics_pb2.MonitoringInfo]]
profiler_factory=None, # type: Optional[Callable[..., Profile]]
- log_lull_timeout_ns=None,
+ log_lull_timeout_ns=None, # type: Optional[int]
):
+ # type: (...) -> None
self.bundle_processor_cache = bundle_processor_cache
self.state_cache_metrics_fn = state_cache_metrics_fn
self.profiler_factory = profiler_factory
self.log_lull_timeout_ns = (
log_lull_timeout_ns or DEFAULT_LOG_LULL_TIMEOUT_NS)
- self._last_full_thread_dump_secs = 0
+ self._last_full_thread_dump_secs = 0.0
Review comment:
this attribute eventually gets set to the value of `time.time()` which is a `float`.
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/8f35da465a195fafcf97ea8e5b7b398131bf605e?el=desc) will **decrease** coverage by `0.15%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.49% 82.33% -0.16%
==========================================
Files 454 456 +2
Lines 54767 54657 -110
==========================================
- Hits 45180 45004 -176
- Misses 9587 9653 +66
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.98% <84.84%> (-1.17%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <85.71%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.11%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [38 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [6fe33a5...e396588](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-703883653
whoops. ok, all lint tests are passing.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491597767
##########
File path: sdks/python/tox.ini
##########
@@ -27,6 +27,8 @@ select = E3
# Shared environment options.
[testenv]
+# allow apps that support color to use it.
+passenv=TERM
Review comment:
Now you can see mypy color output from tox! Let me know if you want me to do this in another 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] chadrik edited a comment on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698675941
> > 23:36:24 apache_beam/runners/worker/data_plane.py:714: error: Incompatible return value type (got "Optional[GrpcClientDataChannel]", expected "GrpcClientDataChannel") [return-value]
>
> Sure this shouldn't return optional as well?
When I do that, it just causes errors elsewhere. I couldn't find a solution. I think there's some implicit guarantee going on here. I'd like to identify where it is and add an assert, a cast, and/or a comment, but I don't know the system well enough.
> > 23:36:24 apache_beam/runners/worker/operations.py:722: error: Argument 1 to "append" of "list" has incompatible type "Tuple[DoOperation, Iterable[SplitResultResidual]]"; expected "Tuple[DoOperation, SplitResultResidual]" [arg-type]
>
> self.dofn_runner.process does return an `Iterable[SplitResultResidual]`
yeah, the problem is that `self.execution_context.delayed_applications` is a list of `Tuple[..., SplitResultResidual]`.
and we're trying to append to that a `Tuple[..., Iterable[SplitResultResidual]]`. Should this be a list of `Union`, or is something wrong here?
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:241: error: Signature of "try_split" incompatible with supertype "Operation" [override]
>
> The supertype returns Optional[Any], which should be compatible with this.
The number of arguments is different.
>
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:978: error: "Operation" has no attribute "add_timer_info" [attr-defined]
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:990: error: "Operation" has no attribute "process_timer" [attr-defined]
>
> These will always be DoOperations.
Does `create_execution_tree` return `OrderedDict[str, operations.DoOperation]`?
>
> > 23:36:24 apache_beam/runners/worker/bundle_processor.py:1568: error: Argument 5 to "StateBackedSideInputMap" has incompatible type "Coder"; expected "WindowedValueCoder" [arg-type]
>
> I think this will always hold, we just don't know it due to having deserialized it with the generic mechanisms.
I changed `BeamTransformFactory.get_input_coders` to return `Dict[str, coders.WindowedValueCoder]`. Does that seem right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robertwb commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r492466948
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -119,10 +125,10 @@ class RunnerIOOperation(operations.Operation):
def __init__(self,
name_context, # type: Union[str, common.NameContext]
- step_name,
+ step_name, # type: Any
Review comment:
Is this not str or Optional[str]?
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -331,6 +369,7 @@ def add_to_inverse_output(timer):
is_last=False))
def close_stream(timer):
+ # type: (bytes) -> None
Review comment:
I wonder if we should call this encoded_timer[s]?
##########
File path: sdks/python/apache_beam/coders/coder_impl.py
##########
@@ -725,7 +726,7 @@ def __init__(self, key_coder_impl, window_coder_impl):
self._tag_coder_impl = StrUtf8Coder().get_impl()
def encode_to_stream(self, value, out, nested):
- # type: (dict, create_OutputStream, bool) -> None
+ # type: (userstate.Timer, create_OutputStream, bool) -> None
Review comment:
I think it used to be correct back when timers were being implemented. This code changed a couple of months ago too.
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -1070,23 +1078,24 @@ def delayed_bundle_application(self,
return beam_fn_api_pb2.DelayedBundleApplication(
requested_time_delay=proto_deferred_watermark,
application=self.construct_bundle_application(
- op, current_watermark, element_and_restriction))
+ op.input_info, current_watermark, element_and_restriction))
def bundle_application(self,
op, # type: operations.DoOperation
primary # type: SplitResultPrimary
):
# type: (...) -> beam_fn_api_pb2.BundleApplication
- return self.construct_bundle_application(op, None, primary.primary_value)
+ assert op.input_info is not None
+ return self.construct_bundle_application(
+ op.input_info, None, primary.primary_value)
def construct_bundle_application(self,
- op, # type: operations.DoOperation
+ op_input_info, # type: operations.OpInputInfo
Review comment:
Sounds good to me.
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -81,7 +85,11 @@
class ClosableOutputStream(OutputStream):
"""A Outputstream for use with CoderImpls that has a close() method."""
- def __init__(self, close_callback=None):
+ def __init__(
+ self,
+ close_callback=None # type: Optional[Optional[Callable[[bytes], None]]]
Review comment:
Why the double Optional (here and elsewhere below)?
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -218,7 +249,7 @@ class DataChannel(with_metaclass(abc.ABCMeta, object)): # type: ignore[misc]
@abc.abstractmethod
def input_elements(self,
instruction_id, # type: str
- expected_inputs, # type: Collection[str]
+ expected_inputs, # type: Sized
Review comment:
Don't we call `__contains__` as well? I'd rather keep it more fully typed.
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -947,11 +955,11 @@ def process_bundle(self, instruction_id):
# (transform_id, timer_family_id).
data_channels = collections.defaultdict(
list
- ) # type: DefaultDict[data_plane.GrpcClientDataChannel, List[str]]
+ ) # type: DefaultDict[data_plane.GrpcClientDataChannel, List[Union[str, Tuple[str, str]]]]
Review comment:
Yep, this changed a couple of months ago. It'll be good to finally have these type annotations checked.
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -243,7 +274,7 @@ def output_stream(
instruction_id, # type: str
transform_id # type: str
):
- # type: (...) -> ClosableOutputStream
+ # type: (...) -> SizeBasedBufferingClosableOutputStream
Review comment:
We're also thinking about adding a time-based one.
Let's add a no-op maybe_flush method to the baseclass and keep ClosableOutputStream everywhere.
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?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 `89.16%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.32% -0.02%
==========================================
Files 452 454 +2
Lines 54016 54109 +93
==========================================
+ Hits 44481 44547 +66
- Misses 9535 9562 +27
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.12% <0.00%> (-0.14%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <85.71%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.07% <89.47%> (-0.51%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [15 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...7f07f86](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491596716
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -1070,23 +1078,24 @@ def delayed_bundle_application(self,
return beam_fn_api_pb2.DelayedBundleApplication(
requested_time_delay=proto_deferred_watermark,
application=self.construct_bundle_application(
- op, current_watermark, element_and_restriction))
+ op.input_info, current_watermark, element_and_restriction))
def bundle_application(self,
op, # type: operations.DoOperation
primary # type: SplitResultPrimary
):
# type: (...) -> beam_fn_api_pb2.BundleApplication
- return self.construct_bundle_application(op, None, primary.primary_value)
+ assert op.input_info is not None
+ return self.construct_bundle_application(
+ op.input_info, None, primary.primary_value)
def construct_bundle_application(self,
- op, # type: operations.DoOperation
+ op_input_info, # type: operations.OpInputInfo
Review comment:
Instead of passing around a `DoOperation` I switched this to a `OpInputInfo` because the latter is all that we need, and doing so let's us avoid having to assert that it is not `None` everywhere that it's used.
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/067cba8229694e7fb9693313f51ca686746b620a?el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `90.00%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.31% -0.04%
==========================================
Files 452 456 +4
Lines 54016 54657 +641
==========================================
+ Hits 44481 44991 +510
- Misses 9535 9666 +131
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.13% <ø> (-0.12%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `88.32% <85.71%> (-1.59%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.34% <90.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [58 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...e1483c9](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491722936
##########
File path: sdks/python/apache_beam/runners/worker/log_handler.py
##########
@@ -156,9 +174,11 @@ def _write_log_entries(self):
done = True
log_entries.pop()
if log_entries:
- yield beam_fn_api_pb2.LogEntry.List(log_entries=log_entries)
+ yield beam_fn_api_pb2.LogEntry.List(
+ log_entries=cast(List[beam_fn_api_pb2.LogEntry], log_entries))
Review comment:
we have to cast here because `log_entries` started out life as a `List[Union[..., Sentinel]]`, and just above this line we checked for the sentinel and popped it out, but mypy can't track that.
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
----------------------------------------------------------------
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] chadrik commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698673308
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491723346
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -1619,7 +1633,7 @@ def _create_pardo_operation(
consumers,
output_tags)
if pardo_proto and pardo_proto.restriction_coder_id:
- result.input_info = (
+ result.input_info = operations.OpInputInfo(
Review comment:
Making this a NamedTuple added a bit of sanity, and should be completely safe.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robertwb commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
robertwb commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698656951
> 23:36:24 apache_beam/runners/worker/sdk_worker.py:665: error: Argument "process_bundle_progress" to "InstructionResponse" has incompatible type "ProcessBundleProgressMetadataResponse"; expected "Optional[ProcessBundleProgressResponse]" [arg-type]
> 23:36:24 apache_beam/runners/worker/sdk_worker.py:667: error: Argument "monitoring_info" to "ProcessBundleProgressMetadataResponse" has incompatible type "List[MonitoringInfo]"; expected "Optional[Mapping[str, Any]]" [arg-type]
@lukecwik this looks wrong, wonder why it's not failing at runtime. Maybe short-ids aren't actually used yet?
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?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 `89.16%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.34% 82.32% -0.02%
==========================================
Files 452 454 +2
Lines 54016 54109 +93
==========================================
+ Hits 44481 44547 +66
- Misses 9535 9562 +27
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/coders/coder\_impl.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vY29kZXJzL2NvZGVyX2ltcGwucHk=) | `95.12% <0.00%> (-0.14%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.29% <84.84%> (-1.05%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <85.71%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.07% <89.47%> (-0.51%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| ... and [15 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [067cba8...f252b9c](https://codecov.io/gh/apache/beam/pull/12881?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] robertwb commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r492466948
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -119,10 +125,10 @@ class RunnerIOOperation(operations.Operation):
def __init__(self,
name_context, # type: Union[str, common.NameContext]
- step_name,
+ step_name, # type: Any
Review comment:
Is this not str or Optional[str]?
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -331,6 +369,7 @@ def add_to_inverse_output(timer):
is_last=False))
def close_stream(timer):
+ # type: (bytes) -> None
Review comment:
I wonder if we should call this encoded_timer[s]?
##########
File path: sdks/python/apache_beam/coders/coder_impl.py
##########
@@ -725,7 +726,7 @@ def __init__(self, key_coder_impl, window_coder_impl):
self._tag_coder_impl = StrUtf8Coder().get_impl()
def encode_to_stream(self, value, out, nested):
- # type: (dict, create_OutputStream, bool) -> None
+ # type: (userstate.Timer, create_OutputStream, bool) -> None
Review comment:
I think it used to be correct back when timers were being implemented. This code changed a couple of months ago too.
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -1070,23 +1078,24 @@ def delayed_bundle_application(self,
return beam_fn_api_pb2.DelayedBundleApplication(
requested_time_delay=proto_deferred_watermark,
application=self.construct_bundle_application(
- op, current_watermark, element_and_restriction))
+ op.input_info, current_watermark, element_and_restriction))
def bundle_application(self,
op, # type: operations.DoOperation
primary # type: SplitResultPrimary
):
# type: (...) -> beam_fn_api_pb2.BundleApplication
- return self.construct_bundle_application(op, None, primary.primary_value)
+ assert op.input_info is not None
+ return self.construct_bundle_application(
+ op.input_info, None, primary.primary_value)
def construct_bundle_application(self,
- op, # type: operations.DoOperation
+ op_input_info, # type: operations.OpInputInfo
Review comment:
Sounds good to me.
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -81,7 +85,11 @@
class ClosableOutputStream(OutputStream):
"""A Outputstream for use with CoderImpls that has a close() method."""
- def __init__(self, close_callback=None):
+ def __init__(
+ self,
+ close_callback=None # type: Optional[Optional[Callable[[bytes], None]]]
Review comment:
Why the double Optional (here and elsewhere below)?
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -218,7 +249,7 @@ class DataChannel(with_metaclass(abc.ABCMeta, object)): # type: ignore[misc]
@abc.abstractmethod
def input_elements(self,
instruction_id, # type: str
- expected_inputs, # type: Collection[str]
+ expected_inputs, # type: Sized
Review comment:
Don't we call `__contains__` as well? I'd rather keep it more fully typed.
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -947,11 +955,11 @@ def process_bundle(self, instruction_id):
# (transform_id, timer_family_id).
data_channels = collections.defaultdict(
list
- ) # type: DefaultDict[data_plane.GrpcClientDataChannel, List[str]]
+ ) # type: DefaultDict[data_plane.GrpcClientDataChannel, List[Union[str, Tuple[str, str]]]]
Review comment:
Yep, this changed a couple of months ago. It'll be good to finally have these type annotations checked.
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -243,7 +274,7 @@ def output_stream(
instruction_id, # type: str
transform_id # type: str
):
- # type: (...) -> ClosableOutputStream
+ # type: (...) -> SizeBasedBufferingClosableOutputStream
Review comment:
We're also thinking about adding a time-based one.
Let's add a no-op maybe_flush method to the baseclass and keep ClosableOutputStream everywhere.
----------------------------------------------------------------
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] chadrik commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-695364802
R: @robertwb
There are 8 errors remaining which I could use a Beam expert to help me resolve.
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/52698a42084100b70cebeb867925859b1c583ec0?el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `91.06%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
- Coverage 82.51% 82.50% -0.02%
==========================================
Files 453 455 +2
Lines 54610 54786 +176
==========================================
+ Hits 45061 45199 +138
- Misses 9549 9587 +38
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/profiler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvcHJvZmlsZXIucHk=) | `86.45% <80.00%> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.47% <86.11%> (-0.68%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <86.66%> (-0.39%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.47% <90.47%> (+0.02%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/doctests.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2RvY3Rlc3RzLnB5) | `96.75% <96.22%> (-0.45%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/filebasedsource.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZmlsZWJhc2Vkc291cmNlLnB5) | `98.77% <100.00%> (+0.02%)` | :arrow_up: |
| ... and [22 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [969b0c6...f6dd36b](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491722936
##########
File path: sdks/python/apache_beam/runners/worker/log_handler.py
##########
@@ -156,9 +174,11 @@ def _write_log_entries(self):
done = True
log_entries.pop()
if log_entries:
- yield beam_fn_api_pb2.LogEntry.List(log_entries=log_entries)
+ yield beam_fn_api_pb2.LogEntry.List(
+ log_entries=cast(List[beam_fn_api_pb2.LogEntry], log_entries))
Review comment:
we have to cast here because `log_entries` was initialized as a `List[Union[..., Sentinel]]`, and just above this line we checked for the sentinel and popped it out, but mypy can't track that.
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r498545227
##########
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##########
@@ -622,10 +659,12 @@ def process_bundle_progress_metadata_request(self,
request, # type: beam_fn_api_pb2.ProcessBundleProgressMetadataRequest
instruction_id # type: str
):
+ # type: (...) -> beam_fn_api_pb2.InstructionResponse
return beam_fn_api_pb2.InstructionResponse(
instruction_id=instruction_id,
- process_bundle_progress=beam_fn_api_pb2.
+ process_bundle_progress_metadata=beam_fn_api_pb2.
ProcessBundleProgressMetadataResponse(
+ # FIXME: incompatible type "List[MonitoringInfo]"; expected "Optional[Mapping[str, Any]]"
monitoring_info=SHORT_ID_CACHE.getInfos(
request.monitoring_info_id)))
Review comment:
This code is not being used currently and it should be a mapping.
Yes, please update getInfos to return that mapping.
----------------------------------------------------------------
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 #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-696540479
# [Codecov](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=h1) Report
> Merging [#12881](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=desc) into [master](https://codecov.io/gh/apache/beam/commit/29787b38b594e29428adaf230b45f9b33e24fa66?el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `90.40%`.
[](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12881 +/- ##
==========================================
+ Coverage 82.48% 82.50% +0.01%
==========================================
Files 454 455 +1
Lines 54813 54786 -27
==========================================
- Hits 45215 45199 -16
+ Misses 9598 9587 -11
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/12881?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/utils/timestamp.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvdGltZXN0YW1wLnB5) | `95.43% <ø> (ø)` | |
| [sdks/python/apache\_beam/utils/windowed\_value.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvd2luZG93ZWRfdmFsdWUucHk=) | `92.54% <ø> (ø)` | |
| [...s/python/apache\_beam/runners/worker/log\_handler.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nX2hhbmRsZXIucHk=) | `87.71% <83.33%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.47% <86.11%> (-0.51%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.52% <86.66%> (+0.83%)` | :arrow_up: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `94.47% <90.47%> (+0.02%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/statecache.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc3RhdGVjYWNoZS5weQ==) | `96.18% <93.75%> (-0.54%)` | :arrow_down: |
| [sdks/python/apache\_beam/runners/worker/logger.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvbG9nZ2VyLnB5) | `93.75% <100.00%> (-0.28%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/opcounters.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3Bjb3VudGVycy5weQ==) | `84.50% <100.00%> (+0.10%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/operations.py](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvb3BlcmF0aW9ucy5weQ==) | `69.34% <100.00%> (+0.16%)` | :arrow_up: |
| ... and [20 more](https://codecov.io/gh/apache/beam/pull/12881/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/12881?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/12881?src=pr&el=footer). Last update [cfe8109...7385641](https://codecov.io/gh/apache/beam/pull/12881?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] chadrik removed a comment on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik removed a comment on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-698673308
> apache_beam/runners/worker/operations.py:722
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r494688827
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -966,6 +976,7 @@ def process_bundle(self, instruction_id):
output_stream = self.timer_data_channel.output_timer_stream(
instruction_id, transform_id, timer_family_id)
timer_info.output_stream = output_stream
+ # FIXME: how do we know that self.ops[transform_id] is a DoOperation?
self.ops[transform_id].add_timer_info(timer_family_id, timer_info)
Review comment:
`BundleProcessor.ops` is `OrderedDict[str, operations.Operation]`. Should it be `OrderedDict[str, operations.DoOperation]`? Or do the transform ids in `BundleProcessor.timers_info` all point to `DoOperations`?
----------------------------------------------------------------
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] chadrik commented on pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on pull request #12881:
URL: https://github.com/apache/beam/pull/12881#issuecomment-702857350
I think the only failures remaining are just flaky tests.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r492493751
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -81,7 +85,11 @@
class ClosableOutputStream(OutputStream):
"""A Outputstream for use with CoderImpls that has a close() method."""
- def __init__(self, close_callback=None):
+ def __init__(
+ self,
+ close_callback=None # type: Optional[Optional[Callable[[bytes], None]]]
Review comment:
mistake. fixed.
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -119,10 +125,10 @@ class RunnerIOOperation(operations.Operation):
def __init__(self,
name_context, # type: Union[str, common.NameContext]
- step_name,
+ step_name, # type: Any
Review comment:
It looks like this argument isn't used at all any more.
The inheritance tree looks like this:
- `operations.Operation`: does not accept a `step_name` argument
- `RunnerIOOperation`: calls `super`. does nothing with `step_name`
- `DataOutputOperation`: does not override `__init__`
- `DataInputOperation`: calls `super`. does nothing with `step_name`
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -243,7 +274,7 @@ def output_stream(
instruction_id, # type: str
transform_id # type: str
):
- # type: (...) -> ClosableOutputStream
+ # type: (...) -> SizeBasedBufferingClosableOutputStream
Review comment:
fixed.
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -218,7 +249,7 @@ class DataChannel(with_metaclass(abc.ABCMeta, object)): # type: ignore[misc]
@abc.abstractmethod
def input_elements(self,
instruction_id, # type: str
- expected_inputs, # type: Collection[str]
+ expected_inputs, # type: Sized
Review comment:
we only call `__len__`, but I'm happy to leave it as a collection. updated.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r491595277
##########
File path: sdks/python/apache_beam/coders/coder_impl.py
##########
@@ -725,7 +726,7 @@ def __init__(self, key_coder_impl, window_coder_impl):
self._tag_coder_impl = StrUtf8Coder().get_impl()
def encode_to_stream(self, value, out, nested):
- # type: (dict, create_OutputStream, bool) -> None
+ # type: (userstate.Timer, create_OutputStream, bool) -> None
Review comment:
as far as I can tell, this was just wrong
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r492498856
##########
File path: sdks/python/apache_beam/runners/worker/data_plane.py
##########
@@ -218,7 +249,7 @@ class DataChannel(with_metaclass(abc.ABCMeta, object)): # type: ignore[misc]
@abc.abstractmethod
def input_elements(self,
instruction_id, # type: str
- expected_inputs, # type: Collection[str]
+ expected_inputs, # type: Sized
Review comment:
we only call `__len__`, but I'm happy to leave it as a collection. updated.
----------------------------------------------------------------
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] chadrik commented on a change in pull request #12881: [BEAM-7746] Get mypy passing on runners.worker
Posted by GitBox <gi...@apache.org>.
chadrik commented on a change in pull request #12881:
URL: https://github.com/apache/beam/pull/12881#discussion_r492495771
##########
File path: sdks/python/apache_beam/runners/worker/bundle_processor.py
##########
@@ -119,10 +125,10 @@ class RunnerIOOperation(operations.Operation):
def __init__(self,
name_context, # type: Union[str, common.NameContext]
- step_name,
+ step_name, # type: Any
Review comment:
It looks like this argument isn't used at all any more.
The inheritance tree looks like this:
- `operations.Operation`: does not accept a `step_name` argument
- `RunnerIOOperation`: calls `super`. does nothing with `step_name`
- `DataOutputOperation`: does not override `__init__`
- `DataInputOperation`: calls `super`. does nothing with `step_name`
----------------------------------------------------------------
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