You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/09/18 17:42:21 UTC
[GitHub] [beam] kennknowles opened a new pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
kennknowles opened a new pull request #12870:
URL: https://github.com/apache/beam/pull/12870
DataflowRunner automatically detects bounded/unbounded collections and will run in batch mode if possible. This forces a run in streaming mode, since this is most of the important coverage.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [x] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [x] Update `CHANGES.md` with noteworthy changes.
- [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website | Whitespace | Typescript
--- | --- | --- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | --- | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12870:
URL: https://github.com/apache/beam/pull/12870#issuecomment-696343825
run dataflow 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] kennknowles commented on pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12870:
URL: https://github.com/apache/beam/pull/12870#issuecomment-696343825
run dataflow 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] kennknowles commented on pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12870:
URL: https://github.com/apache/beam/pull/12870#issuecomment-705858621
OK I think the path of least resistance here is probably to merge first so that the job is established and see how it runs on Jenkins. I will fix the commit history before merge.
----------------------------------------------------------------
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 pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
tysonjh commented on pull request #12870:
URL: https://github.com/apache/beam/pull/12870#issuecomment-705114546
LGTM!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12870:
URL: https://github.com/apache/beam/pull/12870#issuecomment-702933711
R: @tysonjh
Any red flag here? I notice that the batch/automatic `ValidatesRunner` run takes 2.5 hours. Streaming definitely takes longer. I let it run for a couple hours off my laptop this morning but didn't get a complete run yet (due to giving Gradle too much memory and everything crashing down).
----------------------------------------------------------------
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 #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
tysonjh commented on a change in pull request #12870:
URL: https://github.com/apache/beam/pull/12870#discussion_r499922040
##########
File path: runners/google-cloud-dataflow-java/build.gradle
##########
@@ -151,33 +161,34 @@ def commonExcludeCategories = [
// For the following test tasks using legacy worker, set workerHarnessContainerImage to empty to
// make Dataflow pick up the non-versioned container image, which handles a staged worker jar.
-task validatesRunnerLegacyWorkerTest(type: Test) {
- group = "Verification"
- dependsOn ":runners:google-cloud-dataflow-java:worker:legacy-worker:shadowJar"
-
- systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
- "--runner=TestDataflowRunner",
- "--project=${dataflowProject}",
- "--region=${dataflowRegion}",
- "--tempRoot=${dataflowValidatesTempRoot}",
- "--dataflowWorkerJar=${dataflowLegacyWorkerJar}",
- "--workerHarnessContainerImage=",
- ])
-
- // Increase test parallelism up to the number of Gradle workers. By default this is equal
- // to the number of CPU cores, but can be increased by setting --max-workers=N.
- maxParallelForks Integer.MAX_VALUE
- classpath = configurations.validatesRunner
- testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs) +
- files(project(project.path).sourceSets.test.output.classesDirs)
- useJUnit {
- includeCategories 'org.apache.beam.sdk.testing.ValidatesRunner'
- commonExcludeCategories.each {
- excludeCategories it
+def createValidatesRunnerTest = { name, pipelineOptions=legacyPipelineOptions, disabledTests=[] ->
Review comment:
Why change the name to drop `LegacyWorker`? If the plan is to reuse this for other runner versions should the `worker:legacy-worker:shadowJar` dependency be injected somehow as well?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12870:
URL: https://github.com/apache/beam/pull/12870#issuecomment-703755201
Looks like I didn't break the existing `ValidatesRunner` suite, so there's that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] tysonjh commented on a change in pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
tysonjh commented on a change in pull request #12870:
URL: https://github.com/apache/beam/pull/12870#discussion_r500511794
##########
File path: runners/google-cloud-dataflow-java/build.gradle
##########
@@ -151,33 +161,34 @@ def commonExcludeCategories = [
// For the following test tasks using legacy worker, set workerHarnessContainerImage to empty to
// make Dataflow pick up the non-versioned container image, which handles a staged worker jar.
-task validatesRunnerLegacyWorkerTest(type: Test) {
- group = "Verification"
- dependsOn ":runners:google-cloud-dataflow-java:worker:legacy-worker:shadowJar"
-
- systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
- "--runner=TestDataflowRunner",
- "--project=${dataflowProject}",
- "--region=${dataflowRegion}",
- "--tempRoot=${dataflowValidatesTempRoot}",
- "--dataflowWorkerJar=${dataflowLegacyWorkerJar}",
- "--workerHarnessContainerImage=",
- ])
-
- // Increase test parallelism up to the number of Gradle workers. By default this is equal
- // to the number of CPU cores, but can be increased by setting --max-workers=N.
- maxParallelForks Integer.MAX_VALUE
- classpath = configurations.validatesRunner
- testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs) +
- files(project(project.path).sourceSets.test.output.classesDirs)
- useJUnit {
- includeCategories 'org.apache.beam.sdk.testing.ValidatesRunner'
- commonExcludeCategories.each {
- excludeCategories it
+def createValidatesRunnerTest = { name, pipelineOptions=legacyPipelineOptions, disabledTests=[] ->
Review comment:
I meant the name of this method specifically, not the tasks. This method has a dependsOn dependency with the legacy worker, so it will always be for the legacy worker and the naming change seems to make this less clear. Unless of course, in the future, this method could be used for new runners (e.g. runner v2)?
I was considering if it would possible to pull out the `dependsOn ...legacy-worker:shadowJar` (by passing it in as a parameter, specifying the dependency in some other scope, or some other injection mechanism) and then reusing the `createValidatesRunnerTest` method for the runner v2 by passing in `pipelineOptions + ["--experiments=use_runner_v2"]`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12870:
URL: https://github.com/apache/beam/pull/12870#issuecomment-705858621
OK I think the path of least resistance here is probably to merge first so that the job is established and see how it runs on Jenkins. I will fix the commit history before merge.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #12870:
URL: https://github.com/apache/beam/pull/12870#discussion_r500441921
##########
File path: runners/google-cloud-dataflow-java/build.gradle
##########
@@ -151,33 +161,34 @@ def commonExcludeCategories = [
// For the following test tasks using legacy worker, set workerHarnessContainerImage to empty to
// make Dataflow pick up the non-versioned container image, which handles a staged worker jar.
-task validatesRunnerLegacyWorkerTest(type: Test) {
- group = "Verification"
- dependsOn ":runners:google-cloud-dataflow-java:worker:legacy-worker:shadowJar"
-
- systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
- "--runner=TestDataflowRunner",
- "--project=${dataflowProject}",
- "--region=${dataflowRegion}",
- "--tempRoot=${dataflowValidatesTempRoot}",
- "--dataflowWorkerJar=${dataflowLegacyWorkerJar}",
- "--workerHarnessContainerImage=",
- ])
-
- // Increase test parallelism up to the number of Gradle workers. By default this is equal
- // to the number of CPU cores, but can be increased by setting --max-workers=N.
- maxParallelForks Integer.MAX_VALUE
- classpath = configurations.validatesRunner
- testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs) +
- files(project(project.path).sourceSets.test.output.classesDirs)
- useJUnit {
- includeCategories 'org.apache.beam.sdk.testing.ValidatesRunner'
- commonExcludeCategories.each {
- excludeCategories it
+def createValidatesRunnerTest = { name, pipelineOptions=legacyPipelineOptions, disabledTests=[] ->
Review comment:
The actual task name is the same, and defined on line 238. Here I have just made a function for producing variants. The new variant is defined on line 244.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on a change in pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #12870:
URL: https://github.com/apache/beam/pull/12870#discussion_r501132321
##########
File path: runners/google-cloud-dataflow-java/build.gradle
##########
@@ -151,33 +161,34 @@ def commonExcludeCategories = [
// For the following test tasks using legacy worker, set workerHarnessContainerImage to empty to
// make Dataflow pick up the non-versioned container image, which handles a staged worker jar.
-task validatesRunnerLegacyWorkerTest(type: Test) {
- group = "Verification"
- dependsOn ":runners:google-cloud-dataflow-java:worker:legacy-worker:shadowJar"
-
- systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
- "--runner=TestDataflowRunner",
- "--project=${dataflowProject}",
- "--region=${dataflowRegion}",
- "--tempRoot=${dataflowValidatesTempRoot}",
- "--dataflowWorkerJar=${dataflowLegacyWorkerJar}",
- "--workerHarnessContainerImage=",
- ])
-
- // Increase test parallelism up to the number of Gradle workers. By default this is equal
- // to the number of CPU cores, but can be increased by setting --max-workers=N.
- maxParallelForks Integer.MAX_VALUE
- classpath = configurations.validatesRunner
- testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs) +
- files(project(project.path).sourceSets.test.output.classesDirs)
- useJUnit {
- includeCategories 'org.apache.beam.sdk.testing.ValidatesRunner'
- commonExcludeCategories.each {
- excludeCategories it
+def createValidatesRunnerTest = { name, pipelineOptions=legacyPipelineOptions, disabledTests=[] ->
Review comment:
Agree with your naming point, and pushed a rename.
I believe that is possible to continue to configure with a block but Groovy's scope configuration blocks confuse the heck out of me so I did not try to do it. I expect you can do it either by passing the closure in to the `createValidatesRunner` function or by returning something that is ready to accept a block. I'm a bit hazy on how/when the latter works.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12870:
URL: https://github.com/apache/beam/pull/12870#issuecomment-702935673
run dataflow 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] kennknowles merged pull request #12870: [BEAM-2146] Continuously test Dataflow ValidatesRunner forcing streaming
Posted by GitBox <gi...@apache.org>.
kennknowles merged pull request #12870:
URL: https://github.com/apache/beam/pull/12870
----------------------------------------------------------------
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