You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/09/28 00:54:00 UTC
[GitHub] [beam] zhoufek opened a new pull request #15603: [BEAM-9487] Fix AfterProcessingTime and AfterEach
zhoufek opened a new pull request #15603:
URL: https://github.com/apache/beam/pull/15603
This makes two changes to Trigger.may_lose_data implementations:
1. AfterProcessingTime now correctly marks that its condition might not be guaranteed but only if the delay is greater than zero.
2. AfterEach stops being treated the same as AfterAll. The function comment has more details.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [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.
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).
`ValidatesRunner` compliance status (on master branch)
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/badge/icon?subject=V1+Streaming">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon?subject=V1+Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/badge/icon?subject=V2+Streaming">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon?subject=Java+8">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon?subject=Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon?subject=Portable+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/badge/icon?subject=Portable">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon?subject=Structured+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon?subject=ValCont">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
</tbody>
</table>
Examples testing status on various runners
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/badge/icon?subject=V1+Java11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
Post-Commit SDK/Transform Integration Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>Go</th>
<th>Java</th>
<th>Python</th>
</tr>
</thead>
<tbody>
<tr>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon?subject=3.6">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon?subject=3.7">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon?subject=3.8">
</a>
</td>
</tr>
</tbody>
</table>
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>---</th>
<th>Java</th>
<th>Python</th>
<th>Go</th>
<th>Website</th>
<th>Whitespace</th>
<th>Typescript</th>
</tr>
</thead>
<tbody>
<tr>
<td>Non-portable</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon?subject=Tests">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon?subject=Lint">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon?subject=Docker">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/badge/icon?subject=Docs">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Portable</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2b6f56) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60407 +10
=======================================
+ Hits 50601 50615 +14
+ Misses 9796 9792 -4
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.88% <100.00%> (+0.49%)` | :arrow_up: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
| [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `91.03% <0.00%> (-0.36%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.85% <0.00%> (-0.02%)` | :arrow_down: |
| [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
| [sdks/python/apache\_beam/transforms/display.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9kaXNwbGF5LnB5) | `94.03% <0.00%> (+0.08%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.16% <0.00%> (+0.15%)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...e2b6f56](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r727129474
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -850,7 +855,12 @@ class AfterAll(_ParallelTriggerFn):
combine_op = all
def may_lose_data(self, windowing):
- return reduce(or_, (t.may_lose_data(windowing) for t in self.triggers))
+ """If all sub-triggers may finish, then this may finish."""
Review comment:
I'm not sure if there's a difference in how Java handles triggers finishing, but as noted in my other comment, Python's `AfterAll` will only signal that it's finished after all its sub-triggers have signaled that they're finished.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Fix AfterProcessingTime and AfterEach
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...c3eb10b](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head c3eb10b differs from pull request most recent head 0bb51d7. Consider uploading reports for the commit 0bb51d7 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...0bb51d7](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a9708ab) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head a9708ab differs from pull request most recent head 0e9260a. Consider uploading reports for the commit 0e9260a to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
==========================================
- Coverage 83.78% 83.77% -0.01%
==========================================
Files 444 444
Lines 60397 60412 +15
==========================================
+ Hits 50601 50609 +8
- Misses 9796 9803 +7
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.89% <100.00%> (+0.50%)` | :arrow_up: |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5) | `88.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=) | `90.74% <0.00%> (-1.86%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
| [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `91.03% <0.00%> (-0.36%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.85% <0.00%> (-0.02%)` | :arrow_down: |
| [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...0e9260a](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Fix AfterProcessingTime and AfterEach
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Fix AfterProcessingTime and AfterEach
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head c3eb10b differs from pull request most recent head 418447c. Consider uploading reports for the commit 418447c to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...418447c](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r727189577
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -833,13 +839,12 @@ class AfterAny(_ParallelTriggerFn):
combine_op = any
def may_lose_data(self, windowing):
- reason = DataLossReason.NO_POTENTIAL_LOSS
- for trigger in self.triggers:
- t_reason = trigger.may_lose_data(windowing)
- if t_reason == DataLossReason.NO_POTENTIAL_LOSS:
- return t_reason
- reason |= t_reason
- return reason
+ """If any sub-trigger may finish, this one may finish."""
Review comment:
That actually means that its semantics have diverged from spec, so on portable runners where triggers are not executed in the Python we would get different results. Hrmm.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a9708ab) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
==========================================
- Coverage 83.78% 83.77% -0.01%
==========================================
Files 444 444
Lines 60397 60412 +15
==========================================
+ Hits 50601 50609 +8
- Misses 9796 9803 +7
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.89% <100.00%> (+0.50%)` | :arrow_up: |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5) | `88.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=) | `90.74% <0.00%> (-1.86%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
| [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `91.03% <0.00%> (-0.36%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.85% <0.00%> (-0.02%)` | :arrow_down: |
| [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...a9708ab](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r718524059
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Something that still isn't clear to me:
In Beam documentation, we make a note:
```
It is important to note that if, for example, you specify .elementCountAtLeast(50)
and only 32 elements arrive, those 32 elements sit around forever. If the 32 elements
are important to you, consider using composite triggers to combine multiple conditions.
This allows you to specify multiple firing conditions such as “fire either when I receive
50 elements, or every 1 second”.
```
([link](https://beam.apache.org/documentation/programming-guide/#data-driven-triggers))
Based on discussion, though, it sounded like those 32 records would become available at GC time. Is this warning in the documentation only for those who want the data before GC?
Or, putting it in the context of GBK where this is actually used, an [existing test](https://github.com/apache/beam/blob/0111cff88025f0dc783a0890078b769139c8ae36/sdks/python/apache_beam/transforms/ptransform_test.py#L500) shows that `AfterCount(5)` won't emit the data. If I change to `Repeatedly(AfterCount(5))` it has the same behavior, so practically, it seems `Repeatedly(AfterCount(5))` is still unsafe. Should we account for whether or not the windowing is global when making decisions based on GC?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r718524059
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Something that still isn't clear to me:
In Beam documentation, we make a note:
```
It is important to note that if, for example, you specify .elementCountAtLeast(50)
and only 32 elements arrive, those 32 elements sit around forever. If the 32 elements
are important to you, consider using composite triggers to combine multiple conditions.
This allows you to specify multiple firing conditions such as “fire either when I receive
50 elements, or every 1 second”.
```
([link](https://beam.apache.org/documentation/programming-guide/#data-driven-triggers))
Based on discussion, though, it sounded like those 32 records would become available at GC time. Is this warning in the documentation only for those who want the data before GC?
Or, putting it in the context of GBK where this is actually used, an [existing test](https://github.com/apache/beam/blob/0111cff88025f0dc783a0890078b769139c8ae36/sdks/python/apache_beam/transforms/ptransform_test.py#L500) shows that `AfterCount(5)` won't emit the data. If I change to `Repeatedly(AfterCount(5))` it has the same behavior, so practically, it seems `Repeatedly(AfterCount(5))` is still unsafe. Is this something to do with the runner or windowing that wouldn't affect a real pipeline?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Fix AfterProcessingTime and AfterEach
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...c3eb10b](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r718524059
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Something that still isn't clear to me:
In Beam documentation, we make a note:
```
It is important to note that if, for example, you specify .elementCountAtLeast(50)
and only 32 elements arrive, those 32 elements sit around forever. If the 32 elements
are important to you, consider using composite triggers to combine multiple conditions.
This allows you to specify multiple firing conditions such as “fire either when I receive
50 elements, or every 1 second”.
```
([link](https://beam.apache.org/documentation/programming-guide/#data-driven-triggers))
Based on discussion, though, it sounded like those 32 records would become available at GC time. Is this warning in the documentation only for those who want the data before GC?
Or, putting it in the context of GBK where this is actually used, an [existing test](https://github.com/apache/beam/blob/0111cff88025f0dc783a0890078b769139c8ae36/sdks/python/apache_beam/transforms/ptransform_test.py#L500) shows that `AfterCount(5)` won't emit the data. If I change to `Repeatedly(AfterCount(5))` it has the same behavior, so practically, it seems `Repeatedly(AfterCount(5))` is still unsafe. Is this something to do with the runner or windowing that wouldn't affect a real pipeline?
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Something that still isn't clear to me:
In Beam documentation, we make a note:
```
It is important to note that if, for example, you specify .elementCountAtLeast(50)
and only 32 elements arrive, those 32 elements sit around forever. If the 32 elements
are important to you, consider using composite triggers to combine multiple conditions.
This allows you to specify multiple firing conditions such as “fire either when I receive
50 elements, or every 1 second”.
```
([link](https://beam.apache.org/documentation/programming-guide/#data-driven-triggers))
Based on discussion, though, it sounded like those 32 records would become available at GC time. Is this warning in the documentation only for those who want the data before GC?
Or, putting it in the context of GBK where this is actually used, an [existing test](https://github.com/apache/beam/blob/0111cff88025f0dc783a0890078b769139c8ae36/sdks/python/apache_beam/transforms/ptransform_test.py#L500) shows that `AfterCount(5)` won't emit the data. If I change to `Repeatedly(AfterCount(5))` it has the same behavior, so practically, it seems `Repeatedly(AfterCount(5))` is still unsafe. Is this something to do with the runner that won't affect a real pipeline? Does global windowing make this unsafe?
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Something that still isn't clear to me:
In Beam documentation, we make a note:
```
It is important to note that if, for example, you specify .elementCountAtLeast(50)
and only 32 elements arrive, those 32 elements sit around forever. If the 32 elements
are important to you, consider using composite triggers to combine multiple conditions.
This allows you to specify multiple firing conditions such as “fire either when I receive
50 elements, or every 1 second”.
```
([link](https://beam.apache.org/documentation/programming-guide/#data-driven-triggers))
Based on discussion, though, it sounded like those 32 records would become available at GC time. Is this warning in the documentation only for those who want the data before GC?
Or, putting it in the context of GBK where this is actually used, an [existing test](https://github.com/apache/beam/blob/0111cff88025f0dc783a0890078b769139c8ae36/sdks/python/apache_beam/transforms/ptransform_test.py#L500) shows that `AfterCount(5)` won't emit the data. If I change to `Repeatedly(AfterCount(5))` it has the same behavior, so practically, it seems `Repeatedly(AfterCount(5))` is still unsafe. Should we account for whether or not the windowing is global when making decisions based on GC?
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Something that still isn't clear to me:
In Beam documentation, we make a note:
```
It is important to note that if, for example, you specify .elementCountAtLeast(50)
and only 32 elements arrive, those 32 elements sit around forever. If the 32 elements
are important to you, consider using composite triggers to combine multiple conditions.
This allows you to specify multiple firing conditions such as “fire either when I receive
50 elements, or every 1 second”.
```
([link](https://beam.apache.org/documentation/programming-guide/#data-driven-triggers))
Based on discussion, though, it sounded like those 32 records would become available at GC time. Is this warning in the documentation only for those who want the data before GC?
Or, putting it in the context of GBK where this is actually used, an [existing test](https://github.com/apache/beam/blob/0111cff88025f0dc783a0890078b769139c8ae36/sdks/python/apache_beam/transforms/ptransform_test.py#L500) shows that `AfterCount(5)` won't emit the data. If I change to `Repeatedly(AfterCount(5))` it has the same behavior, so practically, it seems `Repeatedly(AfterCount(5))` is still unsafe. Should we account for whether or not the windowing is global when making decisions based on GC?
I'm asking, because I'm wondering if the bug wasn't so much with `Repeatedly` as it was with `AfterCount` not accounting for windowing.
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Ok, so what _should_ be happening in the test is that the four elements in the global window should be emitted at GC time, and GBK should process them before the pipeline finishes, but a bug in the direct runner stops those four elements from emitting. Is that right?
If so, I'm guessing I should remove this as a potential data loss reason. It was in there because of the comment in the documentation and because the test seemed to confirm it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head c3eb10b differs from pull request most recent head b9edf86. Consider uploading reports for the commit b9edf86 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...b9edf86](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r727138056
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -833,13 +839,12 @@ class AfterAny(_ParallelTriggerFn):
combine_op = any
def may_lose_data(self, windowing):
- reason = DataLossReason.NO_POTENTIAL_LOSS
- for trigger in self.triggers:
- t_reason = trigger.may_lose_data(windowing)
- if t_reason == DataLossReason.NO_POTENTIAL_LOSS:
- return t_reason
- reason |= t_reason
- return reason
+ """If any sub-trigger may finish, this one may finish."""
Review comment:
`AfterAny` behaves similarly to `AfterAll` but uses Python's `any` for its combine operation, so it's still possible that `AfterAny` will never finish. For instance, `AfterAny(DefaultTrigger(), Repeatedly(AfterCount(50)))` has the three following possibly inputs to `any`:
```
any([False]) # From DefaultTrigger
any([False]) # From Repeatedly(AfterCount(50))
any([False, False]) # From both
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9343f37) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **decrease** coverage by `0.26%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
==========================================
- Coverage 83.78% 83.51% -0.27%
==========================================
Files 444 445 +1
Lines 60397 61404 +1007
==========================================
+ Hits 50601 51279 +678
- Misses 9796 10125 +329
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.86% <100.00%> (+0.47%)` | :arrow_up: |
| [sdks/python/apache\_beam/io/gcp/bigquery.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5LnB5) | `62.72% <0.00%> (-12.84%)` | :arrow_down: |
| [...ython/apache\_beam/runners/interactive/sql/utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvdXRpbHMucHk=) | `76.09% <0.00%> (-7.91%)` | :arrow_down: |
| [...he\_beam/runners/interactive/sql/beam\_sql\_magics.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvYmVhbV9zcWxfbWFnaWNzLnB5) | `49.75% <0.00%> (-4.79%)` | :arrow_down: |
| [...ython/apache\_beam/io/gcp/bigquery\_read\_internal.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3JlYWRfaW50ZXJuYWwucHk=) | `53.92% <0.00%> (-4.24%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/gcp/bigquery\_tools.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3Rvb2xzLnB5) | `82.91% <0.00%> (-3.82%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.00% <0.00%> (-1.49%)` | :arrow_down: |
| [...eam/runners/portability/fn\_api\_runner/execution.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2V4ZWN1dGlvbi5weQ==) | `91.94% <0.00%> (-1.40%)` | :arrow_down: |
| [...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==) | `89.75% <0.00%> (-1.06%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| ... and [58 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...9343f37](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-930191504
R: @kennknowles
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r723705851
##########
File path: sdks/python/apache_beam/transforms/ptransform_test.py
##########
@@ -505,12 +504,10 @@ def test_group_by_key_allow_unsafe_triggers(self):
| beam.Create([(1, 1), (1, 2), (1, 3), (1, 4)])
| WindowInto(
window.GlobalWindows(),
- trigger=trigger.AfterCount(5),
+ trigger=trigger.AfterCount(4),
accumulation_mode=trigger.AccumulationMode.ACCUMULATING)
| beam.GroupByKey())
- # We need five, but it only has four - Displays how this option is
- # dangerous.
- assert_that(pcoll, is_empty())
+ assert_that(pcoll, equal_to([(1, [1, 2, 3, 4])]))
Review comment:
You ought to be able to get output from `AfterCount(5)`, 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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r723714559
##########
File path: sdks/python/apache_beam/transforms/ptransform_test.py
##########
@@ -505,12 +504,10 @@ def test_group_by_key_allow_unsafe_triggers(self):
| beam.Create([(1, 1), (1, 2), (1, 3), (1, 4)])
| WindowInto(
window.GlobalWindows(),
- trigger=trigger.AfterCount(5),
+ trigger=trigger.AfterCount(4),
accumulation_mode=trigger.AccumulationMode.ACCUMULATING)
| beam.GroupByKey())
- # We need five, but it only has four - Displays how this option is
- # dangerous.
- assert_that(pcoll, is_empty())
+ assert_that(pcoll, equal_to([(1, [1, 2, 3, 4])]))
Review comment:
Yes, but that doesn't currently happen. If I set it to five, I get:
```
apache_beam.testing.util.BeamAssertException: Failed assert: [(1, [1, 2, 3, 4])] == [], missing elements [(1, [1, 2, 3, 4])] [while running 'assert_that/Match']
```
I'm guessing there's a bug with the Direct Runner.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r726571848
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -850,7 +855,12 @@ class AfterAll(_ParallelTriggerFn):
combine_op = all
def may_lose_data(self, windowing):
- return reduce(or_, (t.may_lose_data(windowing) for t in self.triggers))
+ """If all sub-triggers may finish, then this may finish."""
Review comment:
Noted below: I believe this will finish once all subtriggers have _fired_ so you can always return unsafe. https://github.com/apache/beam/blob/d2b785a98cd644e2a683c8895375a3fdc2c96ece/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/windowing/AfterAll.java#L33
https://github.com/apache/beam/blob/d2b785a98cd644e2a683c8895375a3fdc2c96ece/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/windowing/Trigger.java#L245-L247
##########
File path: sdks/python/apache_beam/transforms/trigger_test.py
##########
@@ -470,93 +470,58 @@ def test_after_watermark_no_allowed_lateness_safe_late(self):
0,
DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_watermark_safe_late(self):
+ def test_after_watermark_allowed_lateness_safe_late(self):
self._test(
AfterWatermark(late=DefaultTrigger()),
60,
DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_watermark_no_allowed_lateness_may_finish_late(self):
- self._test(
- AfterWatermark(late=AfterProcessingTime()),
- 0,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_watermark_may_finish_late(self):
- self._test(
- AfterWatermark(late=AfterProcessingTime()),
- 60,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_watermark_no_allowed_lateness_condition_late(self):
- self._test(
- AfterWatermark(late=AfterCount(5)), 0, DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_watermark_condition_late(self):
- self._test(
- AfterWatermark(late=AfterCount(5)),
- 60,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_count_one(self):
- self._test(AfterCount(1), 0, DataLossReason.MAY_FINISH)
-
- def test_after_count_gt_one(self):
- self._test(
- AfterCount(2),
- 0,
- DataLossReason.MAY_FINISH | DataLossReason.CONDITION_NOT_GUARANTEED)
+ def test_after_count(self):
+ self._test(AfterCount(42), 0, DataLossReason.MAY_FINISH)
def test_repeatedly_safe_underlying(self):
self._test(
Repeatedly(DefaultTrigger()), 0, DataLossReason.NO_POTENTIAL_LOSS)
- def test_repeatedly_may_finish_underlying(self):
- self._test(Repeatedly(AfterCount(1)), 0, DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_repeatedly_condition_underlying(self):
- self._test(Repeatedly(AfterCount(2)), 0, DataLossReason.NO_POTENTIAL_LOSS)
+ def test_repeatedly_unsafe_underlying(self):
+ self._test(Repeatedly(AfterCount(42)), 0, DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_any_some_unsafe(self):
+ def test_after_any_one_may_finish(self):
self._test(
- AfterAny(AfterCount(1), DefaultTrigger()),
- 0,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_any_same_reason(self):
- self._test(
- AfterAny(AfterCount(1), AfterProcessingTime()),
+ AfterAny(AfterCount(42), DefaultTrigger()),
0,
DataLossReason.MAY_FINISH)
- def test_after_any_different_reasons(self):
+ def test_after_any_all_safe(self):
self._test(
- AfterAny(AfterCount(2), AfterProcessingTime()),
+ AfterAny(Repeatedly(AfterCount(42)), DefaultTrigger()),
0,
- DataLossReason.MAY_FINISH | DataLossReason.CONDITION_NOT_GUARANTEED)
-
- def test_after_all_some_unsafe(self):
- self._test(
- AfterAll(AfterCount(1), DefaultTrigger()), 0, DataLossReason.MAY_FINISH)
+ DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_all_safe(self):
+ def test_after_all_some_may_finish(self):
self._test(
- AfterAll(Repeatedly(AfterCount(1)), DefaultTrigger()),
+ AfterAll(AfterCount(1), DefaultTrigger()),
Review comment:
AfterAll I believe will finish after DefaultTrigger fires once.
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -833,13 +839,12 @@ class AfterAny(_ParallelTriggerFn):
combine_op = any
def may_lose_data(self, windowing):
- reason = DataLossReason.NO_POTENTIAL_LOSS
- for trigger in self.triggers:
- t_reason = trigger.may_lose_data(windowing)
- if t_reason == DataLossReason.NO_POTENTIAL_LOSS:
- return t_reason
- reason |= t_reason
- return reason
+ """If any sub-trigger may finish, this one may finish."""
Review comment:
This one also is always data loss risk.
https://github.com/apache/beam/blob/d2b785a98cd644e2a683c8895375a3fdc2c96ece/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/windowing/AfterFirst.java#L34
https://github.com/apache/beam/blob/d2b785a98cd644e2a683c8895375a3fdc2c96ece/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/windowing/Trigger.java#L245-L247
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r723705851
##########
File path: sdks/python/apache_beam/transforms/ptransform_test.py
##########
@@ -505,12 +504,10 @@ def test_group_by_key_allow_unsafe_triggers(self):
| beam.Create([(1, 1), (1, 2), (1, 3), (1, 4)])
| WindowInto(
window.GlobalWindows(),
- trigger=trigger.AfterCount(5),
+ trigger=trigger.AfterCount(4),
accumulation_mode=trigger.AccumulationMode.ACCUMULATING)
| beam.GroupByKey())
- # We need five, but it only has four - Displays how this option is
- # dangerous.
- assert_that(pcoll, is_empty())
+ assert_that(pcoll, equal_to([(1, [1, 2, 3, 4])]))
Review comment:
You ought to be able to get output from `AfterCount(5)`, right?
##########
File path: sdks/python/apache_beam/transforms/ptransform_test.py
##########
@@ -505,12 +504,10 @@ def test_group_by_key_allow_unsafe_triggers(self):
| beam.Create([(1, 1), (1, 2), (1, 3), (1, 4)])
| WindowInto(
window.GlobalWindows(),
- trigger=trigger.AfterCount(5),
+ trigger=trigger.AfterCount(4),
accumulation_mode=trigger.AccumulationMode.ACCUMULATING)
| beam.GroupByKey())
- # We need five, but it only has four - Displays how this option is
- # dangerous.
- assert_that(pcoll, is_empty())
+ assert_that(pcoll, equal_to([(1, [1, 2, 3, 4])]))
Review comment:
Yea it sounds like it. We would benefit from:
1. Test of an unsafe trigger that does fire, confirming it is allowed and works.
2. Test of unsafe trigger dropping subsequent data (less important to test)
3. Test of arbitrary trigger (doesn't matter which kind) that never fires but still on GC the elements come out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head c3eb10b differs from pull request most recent head b9edf86. Consider uploading reports for the commit b9edf86 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...b9edf86](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r718838778
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Ok, so what _should_ be happening in the test is that the four elements in the global window should be emitted at GC time, and GBK should process them before the pipeline finishes, but a bug in the direct runner stops those four elements from emitting. Is that right?
If so, I'm guessing I should remove this as a potential data loss reason. It was in there because of the comment in the documentation and because the test seemed to confirm it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ibzib merged pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
ibzib merged pull request #15603:
URL: https://github.com/apache/beam/pull/15603
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-929543975
Accidentally deleted the branch while doing some cleanup. The PR was never intended to be closed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2b6f56) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head e2b6f56 differs from pull request most recent head a9708ab. Consider uploading reports for the commit a9708ab to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60407 +10
=======================================
+ Hits 50601 50615 +14
+ Misses 9796 9792 -4
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.88% <100.00%> (+0.49%)` | :arrow_up: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
| [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `91.03% <0.00%> (-0.36%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.85% <0.00%> (-0.02%)` | :arrow_down: |
| [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
| [sdks/python/apache\_beam/transforms/display.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9kaXNwbGF5LnB5) | `94.03% <0.00%> (+0.08%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.16% <0.00%> (+0.15%)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...a9708ab](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r719683245
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Stopped using the flag and marked is as deprecated. I'm not switching to a bool-returning function or removing the flag, since it has existed for a few versions now and may be in use by some users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Fix AfterProcessingTime and AfterEach
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head c3eb10b differs from pull request most recent head 418447c. Consider uploading reports for the commit 418447c to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...418447c](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a9708ab) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
==========================================
- Coverage 83.78% 83.77% -0.01%
==========================================
Files 444 444
Lines 60397 60412 +15
==========================================
+ Hits 50601 50609 +8
- Misses 9796 9803 +7
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.89% <100.00%> (+0.50%)` | :arrow_up: |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5) | `88.37% <0.00%> (-4.66%)` | :arrow_down: |
| [...che\_beam/runners/interactive/interactive\_runner.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9ydW5uZXIucHk=) | `90.74% <0.00%> (-1.86%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
| [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `91.03% <0.00%> (-0.36%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.85% <0.00%> (-0.02%)` | :arrow_down: |
| [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
| ... and [3 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...a9708ab](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e9260a) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **decrease** coverage by `0.27%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
==========================================
- Coverage 83.78% 83.50% -0.28%
==========================================
Files 444 445 +1
Lines 60397 61188 +791
==========================================
+ Hits 50601 51094 +493
- Misses 9796 10094 +298
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.89% <100.00%> (+0.50%)` | :arrow_up: |
| [sdks/python/apache\_beam/io/gcp/bigquery.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5LnB5) | `62.72% <0.00%> (-12.84%)` | :arrow_down: |
| [...ython/apache\_beam/runners/interactive/sql/utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvdXRpbHMucHk=) | `76.09% <0.00%> (-7.91%)` | :arrow_down: |
| [...he\_beam/runners/interactive/sql/beam\_sql\_magics.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvYmVhbV9zcWxfbWFnaWNzLnB5) | `49.75% <0.00%> (-4.79%)` | :arrow_down: |
| [...ython/apache\_beam/io/gcp/bigquery\_read\_internal.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3JlYWRfaW50ZXJuYWwucHk=) | `53.92% <0.00%> (-4.24%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/gcp/bigquery\_tools.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3Rvb2xzLnB5) | `82.91% <0.00%> (-3.82%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.00% <0.00%> (-1.49%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
| ... and [54 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...0e9260a](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r718524059
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Something that still isn't clear to me:
In Beam documentation, we make a note:
```
It is important to note that if, for example, you specify .elementCountAtLeast(50)
and only 32 elements arrive, those 32 elements sit around forever. If the 32 elements
are important to you, consider using composite triggers to combine multiple conditions.
This allows you to specify multiple firing conditions such as “fire either when I receive
50 elements, or every 1 second”.
```
([link](https://beam.apache.org/documentation/programming-guide/#data-driven-triggers))
Based on discussion, though, it sounded like those 32 records would become available at GC time. Is this warning in the documentation only for those who want the data before GC?
Or, putting it in the context of GBK where this is actually used, an [existing test](https://github.com/apache/beam/blob/0111cff88025f0dc783a0890078b769139c8ae36/sdks/python/apache_beam/transforms/ptransform_test.py#L500) shows that `AfterCount(5)` won't emit the data. If I change to `Repeatedly(AfterCount(5))` it has the same behavior, so practically, it seems `Repeatedly(AfterCount(5))` is still unsafe. Should we account for whether or not the windowing is global when making decisions based on GC?
I'm asking, because I'm wondering if the bug wasn't so much with `Repeatedly` as it was with `AfterCount` not accounting for windowing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r718524059
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Something that still isn't clear to me:
In Beam documentation, we make a note:
```
It is important to note that if, for example, you specify .elementCountAtLeast(50)
and only 32 elements arrive, those 32 elements sit around forever. If the 32 elements
are important to you, consider using composite triggers to combine multiple conditions.
This allows you to specify multiple firing conditions such as “fire either when I receive
50 elements, or every 1 second”.
```
([link](https://beam.apache.org/documentation/programming-guide/#data-driven-triggers))
Based on discussion, though, it sounded like those 32 records would become available at GC time. Is this warning in the documentation only for those who want the data before GC?
Or, putting it in the context of GBK where this is actually used, an [existing test](https://github.com/apache/beam/blob/0111cff88025f0dc783a0890078b769139c8ae36/sdks/python/apache_beam/transforms/ptransform_test.py#L500) shows that `AfterCount(5)` won't emit the data. If I change to `Repeatedly(AfterCount(5))` it has the same behavior, so practically, it seems `Repeatedly(AfterCount(5))` is still unsafe. Is this something to do with the runner that won't affect a real pipeline? Does global windowing make this unsafe?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r718808531
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Yea that comment is outdated. In the current implementation, all buffered elements must be emitted at GC time. Any runner that does not produce those 32 elements as output is broken.
Incidentally, the data loss that unsafe triggers has to do with is not this - it is about if the trigger fires and then further data is just ignored because it "finished".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-930191504
R: @kennknowles
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Fix AfterProcessingTime and AfterEach
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...c3eb10b](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e9260a) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **decrease** coverage by `0.27%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
==========================================
- Coverage 83.78% 83.50% -0.28%
==========================================
Files 444 445 +1
Lines 60397 61188 +791
==========================================
+ Hits 50601 51094 +493
- Misses 9796 10094 +298
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.89% <100.00%> (+0.50%)` | :arrow_up: |
| [sdks/python/apache\_beam/io/gcp/bigquery.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5LnB5) | `62.72% <0.00%> (-12.84%)` | :arrow_down: |
| [...ython/apache\_beam/runners/interactive/sql/utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvdXRpbHMucHk=) | `76.09% <0.00%> (-7.91%)` | :arrow_down: |
| [...he\_beam/runners/interactive/sql/beam\_sql\_magics.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvYmVhbV9zcWxfbWFnaWNzLnB5) | `49.75% <0.00%> (-4.79%)` | :arrow_down: |
| [...ython/apache\_beam/io/gcp/bigquery\_read\_internal.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3JlYWRfaW50ZXJuYWwucHk=) | `53.92% <0.00%> (-4.24%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/gcp/bigquery\_tools.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3Rvb2xzLnB5) | `82.91% <0.00%> (-3.82%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.00% <0.00%> (-1.49%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
| ... and [54 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...0e9260a](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9343f37) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **decrease** coverage by `0.26%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
==========================================
- Coverage 83.78% 83.51% -0.27%
==========================================
Files 444 445 +1
Lines 60397 61404 +1007
==========================================
+ Hits 50601 51279 +678
- Misses 9796 10125 +329
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.86% <100.00%> (+0.47%)` | :arrow_up: |
| [sdks/python/apache\_beam/io/gcp/bigquery.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5LnB5) | `62.72% <0.00%> (-12.84%)` | :arrow_down: |
| [...ython/apache\_beam/runners/interactive/sql/utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvdXRpbHMucHk=) | `76.09% <0.00%> (-7.91%)` | :arrow_down: |
| [...he\_beam/runners/interactive/sql/beam\_sql\_magics.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvYmVhbV9zcWxfbWFnaWNzLnB5) | `49.75% <0.00%> (-4.79%)` | :arrow_down: |
| [...ython/apache\_beam/io/gcp/bigquery\_read\_internal.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3JlYWRfaW50ZXJuYWwucHk=) | `53.92% <0.00%> (-4.24%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/gcp/bigquery\_tools.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3Rvb2xzLnB5) | `82.91% <0.00%> (-3.82%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.00% <0.00%> (-1.49%)` | :arrow_down: |
| [...eam/runners/portability/fn\_api\_runner/execution.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2V4ZWN1dGlvbi5weQ==) | `91.94% <0.00%> (-1.40%)` | :arrow_down: |
| [...eam/runners/portability/fn\_api\_runner/fn\_runner.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2ZuX3J1bm5lci5weQ==) | `89.75% <0.00%> (-1.06%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| ... and [58 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...9343f37](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] commented on pull request #15603: [BEAM-9487] Fix AfterProcessingTime and AfterEach
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...c3eb10b](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-929543975
Accidentally deleted the branch while doing some cleanup. The PR was never intended to be closed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r718808531
##########
File path: sdks/python/apache_beam/transforms/trigger.py
##########
@@ -161,12 +159,42 @@ def with_prefix(self, prefix):
class DataLossReason(Flag):
- """Enum defining potential reasons that a trigger may cause data loss."""
+ """Enum defining potential reasons that a trigger may cause data loss.
+
+ These flags should only cover when the trigger is the cause, though windowing
+ can be taken into account. For instance, AfterWatermark may not flag itself
+ as finishing if the windowing doesn't allow lateness.
+ """
+
+ # Trigger will never be the source of data loss.
NO_POTENTIAL_LOSS = 0
+
+ # Trigger may finish. In this case, data that comes in after the trigger may
+ # be lost. Example: AfterCount(1) will stop firing after the first element.
MAY_FINISH = auto()
+
+ # Trigger has a condition that is not guaranteed to ever be met. In this
+ # case, data that comes in may be lost. Example: AfterCount(42) will lose
+ # 20 records if only 20 come in, since the condition to fire was never met.
CONDITION_NOT_GUARANTEED = auto()
Review comment:
Yea that comment is outdated. In the current implementation, all buffered elements must be emitted at GC time. Any runner that does not produce those 32 elements as output is broken.
Incidentally, the data loss that unsafe triggers has to do with is not this - it is about if the trigger fires and then further data is just ignored because it "finished".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e9260a) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **decrease** coverage by `0.27%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 0e9260a differs from pull request most recent head 9343f37. Consider uploading reports for the commit 9343f37 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
==========================================
- Coverage 83.78% 83.50% -0.28%
==========================================
Files 444 445 +1
Lines 60397 61188 +791
==========================================
+ Hits 50601 51094 +493
- Misses 9796 10094 +298
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.89% <100.00%> (+0.50%)` | :arrow_up: |
| [sdks/python/apache\_beam/io/gcp/bigquery.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5LnB5) | `62.72% <0.00%> (-12.84%)` | :arrow_down: |
| [...ython/apache\_beam/runners/interactive/sql/utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvdXRpbHMucHk=) | `76.09% <0.00%> (-7.91%)` | :arrow_down: |
| [...he\_beam/runners/interactive/sql/beam\_sql\_magics.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9zcWwvYmVhbV9zcWxfbWFnaWNzLnB5) | `49.75% <0.00%> (-4.79%)` | :arrow_down: |
| [...ython/apache\_beam/io/gcp/bigquery\_read\_internal.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3JlYWRfaW50ZXJuYWwucHk=) | `53.92% <0.00%> (-4.24%)` | :arrow_down: |
| [sdks/python/apache\_beam/io/gcp/bigquery\_tools.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW8vZ2NwL2JpZ3F1ZXJ5X3Rvb2xzLnB5) | `82.91% <0.00%> (-3.82%)` | :arrow_down: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.00% <0.00%> (-1.49%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
| ... and [54 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...9343f37](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r727124005
##########
File path: sdks/python/apache_beam/transforms/trigger_test.py
##########
@@ -470,93 +470,58 @@ def test_after_watermark_no_allowed_lateness_safe_late(self):
0,
DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_watermark_safe_late(self):
+ def test_after_watermark_allowed_lateness_safe_late(self):
self._test(
AfterWatermark(late=DefaultTrigger()),
60,
DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_watermark_no_allowed_lateness_may_finish_late(self):
- self._test(
- AfterWatermark(late=AfterProcessingTime()),
- 0,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_watermark_may_finish_late(self):
- self._test(
- AfterWatermark(late=AfterProcessingTime()),
- 60,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_watermark_no_allowed_lateness_condition_late(self):
- self._test(
- AfterWatermark(late=AfterCount(5)), 0, DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_watermark_condition_late(self):
- self._test(
- AfterWatermark(late=AfterCount(5)),
- 60,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_count_one(self):
- self._test(AfterCount(1), 0, DataLossReason.MAY_FINISH)
-
- def test_after_count_gt_one(self):
- self._test(
- AfterCount(2),
- 0,
- DataLossReason.MAY_FINISH | DataLossReason.CONDITION_NOT_GUARANTEED)
+ def test_after_count(self):
+ self._test(AfterCount(42), 0, DataLossReason.MAY_FINISH)
def test_repeatedly_safe_underlying(self):
self._test(
Repeatedly(DefaultTrigger()), 0, DataLossReason.NO_POTENTIAL_LOSS)
- def test_repeatedly_may_finish_underlying(self):
- self._test(Repeatedly(AfterCount(1)), 0, DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_repeatedly_condition_underlying(self):
- self._test(Repeatedly(AfterCount(2)), 0, DataLossReason.NO_POTENTIAL_LOSS)
+ def test_repeatedly_unsafe_underlying(self):
+ self._test(Repeatedly(AfterCount(42)), 0, DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_any_some_unsafe(self):
+ def test_after_any_one_may_finish(self):
self._test(
- AfterAny(AfterCount(1), DefaultTrigger()),
- 0,
- DataLossReason.NO_POTENTIAL_LOSS)
-
- def test_after_any_same_reason(self):
- self._test(
- AfterAny(AfterCount(1), AfterProcessingTime()),
+ AfterAny(AfterCount(42), DefaultTrigger()),
0,
DataLossReason.MAY_FINISH)
- def test_after_any_different_reasons(self):
+ def test_after_any_all_safe(self):
self._test(
- AfterAny(AfterCount(2), AfterProcessingTime()),
+ AfterAny(Repeatedly(AfterCount(42)), DefaultTrigger()),
0,
- DataLossReason.MAY_FINISH | DataLossReason.CONDITION_NOT_GUARANTEED)
-
- def test_after_all_some_unsafe(self):
- self._test(
- AfterAll(AfterCount(1), DefaultTrigger()), 0, DataLossReason.MAY_FINISH)
+ DataLossReason.NO_POTENTIAL_LOSS)
- def test_after_all_safe(self):
+ def test_after_all_some_may_finish(self):
self._test(
- AfterAll(Repeatedly(AfterCount(1)), DefaultTrigger()),
+ AfterAll(AfterCount(1), DefaultTrigger()),
Review comment:
I don't think it will? `_ParallelTriggerFn.on_fire` takes a "combine_op" of all the sub-triggers' `TriggerFn.on_fire` results. For `AfterAll`, that's Python's `all` function. Since `AfterCount` always returns True[1] and `DefaultTrigger` always returns False[2], this would be equivalent to `all([True, False])`, which always returns False, meaning this instance of `AfterAll` will never finish.
[1] https://github.com/apache/beam/blob/ef4364519c94e4bff83bb0f32aace95c3093ad16/sdks/python/apache_beam/transforms/trigger.py#L670-L671
[2] https://github.com/apache/beam/blob/ef4364519c94e4bff83bb0f32aace95c3093ad16/sdks/python/apache_beam/transforms/trigger.py#L335-L336
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-946102228
Opened BEAM-13076 to address the AfterAny/AfterAll issue. Unsure if AfterEach should be included.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head c3eb10b differs from pull request most recent head e2b6f56. Consider uploading reports for the commit e2b6f56 to get more accurate results
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...e2b6f56](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r725290203
##########
File path: sdks/python/apache_beam/transforms/ptransform_test.py
##########
@@ -505,12 +504,10 @@ def test_group_by_key_allow_unsafe_triggers(self):
| beam.Create([(1, 1), (1, 2), (1, 3), (1, 4)])
| WindowInto(
window.GlobalWindows(),
- trigger=trigger.AfterCount(5),
+ trigger=trigger.AfterCount(4),
accumulation_mode=trigger.AccumulationMode.ACCUMULATING)
| beam.GroupByKey())
- # We need five, but it only has four - Displays how this option is
- # dangerous.
- assert_that(pcoll, is_empty())
+ assert_that(pcoll, equal_to([(1, [1, 2, 3, 4])]))
Review comment:
For number 1, we have plenty of tests by now that use it.
For number 2, I added `test_after_count_streaming` to test_trigger.py. I'm putting it there since it is less about testing GBK when unsafe triggers are allowed and more about showing `AfterCount` behavior, including its potential to drop data after firing once.
For number 3, it should be covered by `test_after_count` [here](https://github.com/apache/beam/blob/2bcfb68483bba88cb1951ed175de60ff848abb28/sdks/python/apache_beam/transforms/trigger_test.py#L578), but as a comment notes, it's currently only succeeding because of the Direct Runner bug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek closed pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek closed pull request #15603:
URL: https://github.com/apache/beam/pull/15603
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] edited a comment on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2b6f56) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60407 +10
=======================================
+ Hits 50601 50615 +14
+ Misses 9796 9792 -4
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.88% <100.00%> (+0.49%)` | :arrow_up: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...pache\_beam/runners/interactive/interactive\_beam.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9iZWFtLnB5) | `75.60% <0.00%> (-0.98%)` | :arrow_down: |
| [...runners/interactive/display/pcoll\_visualization.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9kaXNwbGF5L3Bjb2xsX3Zpc3VhbGl6YXRpb24ucHk=) | `85.85% <0.00%> (-0.51%)` | :arrow_down: |
| [...eam/runners/interactive/interactive\_environment.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9pbnRlcmFjdGl2ZS9pbnRlcmFjdGl2ZV9lbnZpcm9ubWVudC5weQ==) | `91.03% <0.00%> (-0.36%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.85% <0.00%> (-0.02%)` | :arrow_down: |
| [...apache\_beam/portability/api/beam\_runner\_api\_pb2.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcG9ydGFiaWxpdHkvYXBpL2JlYW1fcnVubmVyX2FwaV9wYjIucHk=) | `100.00% <0.00%> (ø)` | |
| [sdks/python/apache\_beam/transforms/display.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9kaXNwbGF5LnB5) | `94.03% <0.00%> (+0.08%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.16% <0.00%> (+0.15%)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...e2b6f56](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r724478753
##########
File path: sdks/python/apache_beam/transforms/ptransform_test.py
##########
@@ -505,12 +504,10 @@ def test_group_by_key_allow_unsafe_triggers(self):
| beam.Create([(1, 1), (1, 2), (1, 3), (1, 4)])
| WindowInto(
window.GlobalWindows(),
- trigger=trigger.AfterCount(5),
+ trigger=trigger.AfterCount(4),
accumulation_mode=trigger.AccumulationMode.ACCUMULATING)
| beam.GroupByKey())
- # We need five, but it only has four - Displays how this option is
- # dangerous.
- assert_that(pcoll, is_empty())
+ assert_that(pcoll, equal_to([(1, [1, 2, 3, 4])]))
Review comment:
Yea it sounds like it. We would benefit from:
1. Test of an unsafe trigger that does fire, confirming it is allowed and works.
2. Test of unsafe trigger dropping subsequent data (less important to test)
3. Test of arbitrary trigger (doesn't matter which kind) that never fires but still on GC the elements come out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r723714559
##########
File path: sdks/python/apache_beam/transforms/ptransform_test.py
##########
@@ -505,12 +504,10 @@ def test_group_by_key_allow_unsafe_triggers(self):
| beam.Create([(1, 1), (1, 2), (1, 3), (1, 4)])
| WindowInto(
window.GlobalWindows(),
- trigger=trigger.AfterCount(5),
+ trigger=trigger.AfterCount(4),
accumulation_mode=trigger.AccumulationMode.ACCUMULATING)
| beam.GroupByKey())
- # We need five, but it only has four - Displays how this option is
- # dangerous.
- assert_that(pcoll, is_empty())
+ assert_that(pcoll, equal_to([(1, [1, 2, 3, 4])]))
Review comment:
Yes, but that doesn't currently happen. If I set it to five, I get:
```
apache_beam.testing.util.BeamAssertException: Failed assert: [(1, [1, 2, 3, 4])] == [], missing elements [(1, [1, 2, 3, 4])] [while running 'assert_that/Match']
```
I'm guessing there's a bug with the Direct Runner.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-941032675
I've updated `_ParallelTriggerFn` to implement `may_lose_data` so that `AfterAll` and `AfterAny` inherit it. Like its implementation of `should_fire` and `on_fire`, it bases the decision on `_ParallelTriggerFn.combine_op`, which are set in the implementations. This should make it clearer how `may_lose_data` relates to whether or not a trigger may finish, specifically to the return value of `on_fire`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek closed pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek closed pull request #15603:
URL: https://github.com/apache/beam/pull/15603
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] commented on pull request #15603: [BEAM-9487] Fix AfterProcessingTime and AfterEach
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-928556229
# [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15603](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c3eb10b) into [master](https://codecov.io/gh/apache/beam/commit/e11b80c5a91f304cc4e0a5e9581dfda687f34478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e11b80c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15603 +/- ##
=======================================
Coverage 83.78% 83.78%
=======================================
Files 444 444
Lines 60397 60414 +17
=======================================
+ Hits 50601 50618 +17
Misses 9796 9796
```
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/transforms/trigger.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy90cmlnZ2VyLnB5) | `89.48% <100.00%> (+0.09%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `92.68% <0.00%> (-2.44%)` | :arrow_down: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `90.42% <0.00%> (-1.07%)` | :arrow_down: |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.26% <0.00%> (-0.25%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `88.85% <0.00%> (-0.16%)` | :arrow_down: |
| [sdks/python/apache\_beam/dataframe/frames.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `94.78% <0.00%> (-0.09%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/data\_plane.py](https://codecov.io/gh/apache/beam/pull/15603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvZGF0YV9wbGFuZS5weQ==) | `89.48% <0.00%> (+1.98%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e11b80c...c3eb10b](https://codecov.io/gh/apache/beam/pull/15603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] zhoufek commented on pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes
Posted by GitBox <gi...@apache.org>.
zhoufek commented on pull request #15603:
URL: https://github.com/apache/beam/pull/15603#issuecomment-946105947
Also opened BEAM-13078 to address the Direct Runner bug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org