You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/08/18 17:55:06 UTC
[GitHub] [beam] lukecwik opened a new pull request #12616: [WIP][BEAM-10670] Update Samza to be opt-out for SplittableDoFn powering the Read transform.
lukecwik opened a new pull request #12616:
URL: https://github.com/apache/beam/pull/12616
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik edited a comment on pull request #12616: [BEAM-10670] Update Jet to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik edited a comment on pull request #12616:
URL: https://github.com/apache/beam/pull/12616#issuecomment-675627530
R: @jbartok This passes the validateRunner tests that are part of Jet even after enabling Impulse and bounded splittable DoFn.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tysonjh commented on a change in pull request #12616: [BEAM-10670] Update Jet to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
tysonjh commented on a change in pull request #12616:
URL: https://github.com/apache/beam/pull/12616#discussion_r486679831
##########
File path: runners/jet/build.gradle
##########
@@ -85,6 +84,12 @@ task validatesRunnerBatch(type: Test) {
excludeCategories 'org.apache.beam.sdk.testing.UsesTestStreamWithProcessingTime'
}
filter {
+ // There is an existing bug caused by Jet assuming that outputs emitted are Object.equals comparable which is not true for byte[]
+ excludeTestsMatching 'org.apache.beam.sdk.transforms.SplittableDoFnTest.testOutputAfterCheckpointBounded'
+
+ // There is an existing bug where we are trying to add additional outputs during flusing and don't restore state correctly
Review comment:
```suggestion
// There is an existing bug where we are trying to add additional outputs during flushing and don't restore state correctly
```
##########
File path: runners/jet/src/main/java/org/apache/beam/runners/jet/JetRunner.java
##########
@@ -186,7 +189,15 @@ private JetInstance getJetInstance(JetPipelineOptions options) {
}
private static List<PTransformOverride> getDefaultOverrides() {
- return Collections.emptyList();
+ return Arrays.asList(
Review comment:
Is this list exhaustive? How can I tell if something is missing?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #12616: [BEAM-10670] Update Jet to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12616:
URL: https://github.com/apache/beam/pull/12616#issuecomment-692196137
> What kind of test coverage is there for these transform replacements on the Jet runner?
The `@ValidatesRunner` junit category gives us coverage. The `@ValidatesRunner` tests within https://github.com/apache/beam/blob/master/sdks/java/core/src/test/java/org/apache/beam/sdk/transforms/SplittableDoFnTest.java are comprehensive for this feature but Jet can't support them all due to lack of features and or/bugs (e.g. support for bundle finalization, assuming byte[] is equals comparable, ...).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on a change in pull request #12616: [BEAM-10670] Update Jet to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #12616:
URL: https://github.com/apache/beam/pull/12616#discussion_r488095723
##########
File path: runners/jet/src/main/java/org/apache/beam/runners/jet/JetRunner.java
##########
@@ -186,7 +189,15 @@ private JetInstance getJetInstance(JetPipelineOptions options) {
}
private static List<PTransformOverride> getDefaultOverrides() {
- return Collections.emptyList();
+ return Arrays.asList(
Review comment:
There is no exhaustive list as each runner can choose to implement each transform however they wish. We have to rely on the `@ValidatesRunner` suite of tests for validation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #12616: [BEAM-10670] Update Jet to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12616:
URL: https://github.com/apache/beam/pull/12616#issuecomment-690739301
Run Jet ValidatesRunner
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik merged pull request #12616: [BEAM-10670] Update Jet to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #12616:
URL: https://github.com/apache/beam/pull/12616
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #12616: [WIP][BEAM-10670] Update Samza to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12616:
URL: https://github.com/apache/beam/pull/12616#issuecomment-675627530
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #12616: [WIP][BEAM-10670] Update Jet to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12616:
URL: https://github.com/apache/beam/pull/12616#issuecomment-675673743
Run Java PreCommit
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik commented on pull request #12616: [BEAM-10670] Update Jet to be opt-out for SplittableDoFn powering the Read transform.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on pull request #12616:
URL: https://github.com/apache/beam/pull/12616#issuecomment-690739360
R: @tysonjh
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org