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/29 22:56:03 UTC
[GitHub] [beam] kileys opened a new pull request #12970: [BEAM-8024] Add dataflow and flink runner to JPMS test
kileys opened a new pull request #12970:
URL: https://github.com/apache/beam/pull/12970
Run JPMS test on dataflow and flink runner as well
------------------------
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://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](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_Python_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python_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 a change in pull request #12970: [BEAM-8024] Add dataflow and flink runner to JPMS test
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #12970:
URL: https://github.com/apache/beam/pull/12970#discussion_r497145208
##########
File path: sdks/java/testing/jpms-tests/build.gradle
##########
@@ -31,13 +34,66 @@ enableJavaPerformanceTesting()
description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
ext.summary = "E2E test for Java 9 modules"
+/*
+ * List of runners to run post commit tests on.
+ */
+def postCommitRunnerClass = [
+ directRunner: "org.apache.beam.runners.direct.DirectRunner",
+ flinkRunner: "org.apache.beam.runners.flink.TestFlinkRunner",
+ dataflowRunner: "org.apache.beam.runners.dataflow.TestDataflowRunner",
+]
+for (String runner : postCommitRunnerClass.keySet()) {
+ configurations.create(runner + "PostCommit")
+}
+
dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
- compile project(path: ":runners:direct-java")
compile project(path: ":sdks:java:extensions:google-cloud-platform-core")
testCompile library.java.junit
testCompile library.java.hamcrest_core
+
+ // Add dependencies for the PostCommit configurations
+ // For each runner a project level dependency on the test project.
+ for (String runner : postCommitRunnerClass.keySet()) {
+ delegate.add(runner + "PostCommit", project(":sdks:java:testing:jpms-tests"))
+ delegate.add(runner + "PostCommit", project(path: ":sdks:java:testing:jpms-tests", configuration: "testRuntime"))
+ }
+ directRunnerPostCommit project(":runners:direct-java")
Review comment:
Especially considering this bit, the looping over configuration doesn't really save you much. Could be simpler to just define 3 configs with very straightforward control flow and data flow.
##########
File path: .test-infra/jenkins/job_PostCommit_Java_Jpms_Java11.groovy
##########
@@ -39,7 +39,7 @@ PostcommitJobBuilder.postCommitJob('beam_PostCommit_Java_Jpms_Java11', 'Run Jpms
steps {
gradle {
rootBuildScriptDir(commonJobProperties.checkoutDir)
- tasks(':sdks:java:testing:jpms-tests:integrationTest')
+ tasks(':sdks:java:testing:jpms-tests:postCommit')
Review comment:
Noted below that coupling the various runners has some drawbacks. It is some boilerplate but can we factor it out into a few jobs: direct (candidate for precommit), flink (also candidate, but slower), dataflow (too slow for precommit).
##########
File path: sdks/java/testing/jpms-tests/build.gradle
##########
@@ -31,13 +34,66 @@ enableJavaPerformanceTesting()
description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
ext.summary = "E2E test for Java 9 modules"
+/*
+ * List of runners to run post commit tests on.
+ */
+def postCommitRunnerClass = [
+ directRunner: "org.apache.beam.runners.direct.DirectRunner",
+ flinkRunner: "org.apache.beam.runners.flink.TestFlinkRunner",
+ dataflowRunner: "org.apache.beam.runners.dataflow.TestDataflowRunner",
+]
+for (String runner : postCommitRunnerClass.keySet()) {
Review comment:
Aesthetically, I dislike building code identifiers (or similar) by string concat. Pragmatically it makes it hard to search for them. But looping out these configs makes sense. What do you think about something like a map from configuration name to runner class.
```
def postCommitConfigurations = {
"directRunnerPostCommit" : "org.apache.beam.runners.direct.DirectRunner",
"flinkRunnerPostCommit" : "org.apache.beam.runners.flink.TestFlinkRunner"
"dataflowRunnerPostCommit" : "org.apache.beam.runners.dataflow.TestDataflowRunner"
}
```
##########
File path: sdks/java/testing/jpms-tests/build.gradle
##########
@@ -31,13 +34,66 @@ enableJavaPerformanceTesting()
description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
ext.summary = "E2E test for Java 9 modules"
+/*
+ * List of runners to run post commit tests on.
+ */
+def postCommitRunnerClass = [
+ directRunner: "org.apache.beam.runners.direct.DirectRunner",
+ flinkRunner: "org.apache.beam.runners.flink.TestFlinkRunner",
+ dataflowRunner: "org.apache.beam.runners.dataflow.TestDataflowRunner",
+]
+for (String runner : postCommitRunnerClass.keySet()) {
+ configurations.create(runner + "PostCommit")
+}
+
dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
- compile project(path: ":runners:direct-java")
compile project(path: ":sdks:java:extensions:google-cloud-platform-core")
testCompile library.java.junit
testCompile library.java.hamcrest_core
+
+ // Add dependencies for the PostCommit configurations
+ // For each runner a project level dependency on the test project.
+ for (String runner : postCommitRunnerClass.keySet()) {
+ delegate.add(runner + "PostCommit", project(":sdks:java:testing:jpms-tests"))
+ delegate.add(runner + "PostCommit", project(path: ":sdks:java:testing:jpms-tests", configuration: "testRuntime"))
+ }
+ directRunnerPostCommit project(":runners:direct-java")
+ flinkRunnerPostCommit project(":runners:flink:1.10")
+ dataflowRunnerPostCommit project(":runners:google-cloud-dataflow-java")
+}
+
+/*
+ * Create a ${runner}PostCommit task for each runner which runs a set
+ * of integration tests for WordCount and WindowedWordCount.
+ */
+def gcpProject = project.findProperty('gcpProject') ?: 'apache-beam-testing'
Review comment:
putting a pin in the fact that this is largely independent of your change and applies cross module (no action required)
##########
File path: sdks/java/testing/jpms-tests/build.gradle
##########
@@ -31,13 +34,66 @@ enableJavaPerformanceTesting()
description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
ext.summary = "E2E test for Java 9 modules"
+/*
+ * List of runners to run post commit tests on.
+ */
+def postCommitRunnerClass = [
+ directRunner: "org.apache.beam.runners.direct.DirectRunner",
+ flinkRunner: "org.apache.beam.runners.flink.TestFlinkRunner",
+ dataflowRunner: "org.apache.beam.runners.dataflow.TestDataflowRunner",
+]
+for (String runner : postCommitRunnerClass.keySet()) {
+ configurations.create(runner + "PostCommit")
+}
+
dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
- compile project(path: ":runners:direct-java")
compile project(path: ":sdks:java:extensions:google-cloud-platform-core")
testCompile library.java.junit
testCompile library.java.hamcrest_core
+
+ // Add dependencies for the PostCommit configurations
+ // For each runner a project level dependency on the test project.
+ for (String runner : postCommitRunnerClass.keySet()) {
+ delegate.add(runner + "PostCommit", project(":sdks:java:testing:jpms-tests"))
+ delegate.add(runner + "PostCommit", project(path: ":sdks:java:testing:jpms-tests", configuration: "testRuntime"))
+ }
+ directRunnerPostCommit project(":runners:direct-java")
+ flinkRunnerPostCommit project(":runners:flink:1.10")
+ dataflowRunnerPostCommit project(":runners:google-cloud-dataflow-java")
+}
+
+/*
+ * Create a ${runner}PostCommit task for each runner which runs a set
+ * of integration tests for WordCount and WindowedWordCount.
+ */
+def gcpProject = project.findProperty('gcpProject') ?: 'apache-beam-testing'
+def gcpRegion = project.findProperty('gcpRegion') ?: 'us-central1'
+def gcsTempRoot = project.findProperty('gcsTempRoot') ?: 'gs://temp-storage-for-end-to-end-tests/'
+
+for (String runner : postCommitRunnerClass.keySet()) {
+ tasks.create(name: runner + "PostCommit", type: Test) {
+ def postCommitBeamTestPipelineOptions = [
+ "--project=${gcpProject}",
+ "--tempRoot=${gcsTempRoot}",
+ "--runner=" + postCommitRunnerClass[runner],
+ ]
+ if ("dataflowRunner".equals(runner)) {
+ postCommitBeamTestPipelineOptions.add("--region=${gcpRegion}")
+ }
+ classpath = configurations."${runner}PostCommit"
+ include "**/*IT.class"
+ maxParallelForks 4
+ systemProperty "beamTestPipelineOptions", JsonOutput.toJson(postCommitBeamTestPipelineOptions)
+ }
+}
+
+/* Define a common postcommit task which depends on all the individual postcommits. */
Review comment:
This is a laudable instinct, but I actually don't favor it. Reasons:
1. Prefer to separate test suites from when they may run
2. Runners often have fairly different infrastructures to spin up and potentially blow memory if accidentally run together
3. We want separate signals about what is unhealthy.
Regarding naming of a test suite "postcommit" I want to acknowledge that you are noticing how things are currently organized and following those conventions. So that's sensible. But it is a convention I think we have to move away from. Gradle can be run from lots of places and the notion of precommit vs postcommit is inessential.
----------------------------------------------------------------
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] kileys commented on pull request #12970: [BEAM-8024] Add dataflow and flink runner to JPMS test
Posted by GitBox <gi...@apache.org>.
kileys commented on pull request #12970:
URL: https://github.com/apache/beam/pull/12970#issuecomment-701041474
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on a change in pull request #12970: [BEAM-8024] Add dataflow and flink runner to JPMS test
Posted by GitBox <gi...@apache.org>.
kileys commented on a change in pull request #12970:
URL: https://github.com/apache/beam/pull/12970#discussion_r497962184
##########
File path: sdks/java/testing/jpms-tests/build.gradle
##########
@@ -31,13 +34,66 @@ enableJavaPerformanceTesting()
description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
ext.summary = "E2E test for Java 9 modules"
+/*
+ * List of runners to run post commit tests on.
+ */
+def postCommitRunnerClass = [
+ directRunner: "org.apache.beam.runners.direct.DirectRunner",
+ flinkRunner: "org.apache.beam.runners.flink.TestFlinkRunner",
+ dataflowRunner: "org.apache.beam.runners.dataflow.TestDataflowRunner",
+]
+for (String runner : postCommitRunnerClass.keySet()) {
+ configurations.create(runner + "PostCommit")
+}
+
dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
- compile project(path: ":runners:direct-java")
compile project(path: ":sdks:java:extensions:google-cloud-platform-core")
testCompile library.java.junit
testCompile library.java.hamcrest_core
+
+ // Add dependencies for the PostCommit configurations
+ // For each runner a project level dependency on the test project.
+ for (String runner : postCommitRunnerClass.keySet()) {
+ delegate.add(runner + "PostCommit", project(":sdks:java:testing:jpms-tests"))
+ delegate.add(runner + "PostCommit", project(path: ":sdks:java:testing:jpms-tests", configuration: "testRuntime"))
+ }
+ directRunnerPostCommit project(":runners:direct-java")
+ flinkRunnerPostCommit project(":runners:flink:1.10")
+ dataflowRunnerPostCommit project(":runners:google-cloud-dataflow-java")
+}
+
+/*
+ * Create a ${runner}PostCommit task for each runner which runs a set
+ * of integration tests for WordCount and WindowedWordCount.
+ */
+def gcpProject = project.findProperty('gcpProject') ?: 'apache-beam-testing'
+def gcpRegion = project.findProperty('gcpRegion') ?: 'us-central1'
+def gcsTempRoot = project.findProperty('gcsTempRoot') ?: 'gs://temp-storage-for-end-to-end-tests/'
+
+for (String runner : postCommitRunnerClass.keySet()) {
+ tasks.create(name: runner + "PostCommit", type: Test) {
+ def postCommitBeamTestPipelineOptions = [
+ "--project=${gcpProject}",
+ "--tempRoot=${gcsTempRoot}",
+ "--runner=" + postCommitRunnerClass[runner],
+ ]
+ if ("dataflowRunner".equals(runner)) {
+ postCommitBeamTestPipelineOptions.add("--region=${gcpRegion}")
+ }
+ classpath = configurations."${runner}PostCommit"
+ include "**/*IT.class"
+ maxParallelForks 4
+ systemProperty "beamTestPipelineOptions", JsonOutput.toJson(postCommitBeamTestPipelineOptions)
+ }
+}
+
+/* Define a common postcommit task which depends on all the individual postcommits. */
Review comment:
Makes sense, changed the post commit references to integration test
----------------------------------------------------------------
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 #12970: [BEAM-8024] Add dataflow and flink runner to JPMS test
Posted by GitBox <gi...@apache.org>.
kennknowles merged pull request #12970:
URL: https://github.com/apache/beam/pull/12970
----------------------------------------------------------------
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