You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/09 10:52:01 UTC

[GitHub] [beam] pawelpasterz opened a new pull request #11076: Run precommit portability on java 11

pawelpasterz opened a new pull request #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076
 
 
   **Please** add a meaningful description for your change here
   
   ------------------------
   
   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 | Apex | Dataflow | Flink | Gearpump | Samza | Spark
   --- | --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
   Java | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
   Python | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
   XLang | --- | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) 
   Portable | --- | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.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.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] mwalenia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
mwalenia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-596475071
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] iemejia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
iemejia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598812820
 
 
   Is running the examples with Java 11 supposed to be part of this too? This is separate for the Java 8 case.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] mwalenia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
mwalenia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598690869
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] mwalenia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
mwalenia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-596457058
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] kamilwu commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
kamilwu commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598811011
 
 
   Run Python 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


With regards,
Apache Git Services

[GitHub] [beam] kamilwu commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
kamilwu commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598783152
 
 
   Run JavaPortabilityApiJava11 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


With regards,
Apache Git Services

[GitHub] [beam] mwalenia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
mwalenia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-596473754
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] Ardagan commented on a change in pull request #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
Ardagan commented on a change in pull request #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#discussion_r391111373
 
 

 ##########
 File path: build.gradle
 ##########
 @@ -335,3 +335,27 @@ if (project.hasProperty('javaLinkageArtifactIds')) {
     }
   }
 }
+
+if (project.hasProperty('runWithJava11')) {
+  allprojects {
+    plugins.withId('java') {
+
+      java {
+        sourceCompatibility = JavaVersion.VERSION_1_8
+        targetCompatibility = JavaVersion.VERSION_1_8
+      }
+
+      tasks.withType(JavaCompile) {
+        options.with {
+          fork = true
+          forkOptions.javaHome = java8Home as File
+        }
+      }
+
+      test {
 
 Review comment:
   Will this compile tests with Java11 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


With regards,
Apache Git Services

[GitHub] [beam] pawelpasterz commented on a change in pull request #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
pawelpasterz commented on a change in pull request #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#discussion_r391225981
 
 

 ##########
 File path: build.gradle
 ##########
 @@ -335,3 +335,27 @@ if (project.hasProperty('javaLinkageArtifactIds')) {
     }
   }
 }
+
+if (project.hasProperty('runWithJava11')) {
+  allprojects {
+    plugins.withId('java') {
+
+      java {
+        sourceCompatibility = JavaVersion.VERSION_1_8
+        targetCompatibility = JavaVersion.VERSION_1_8
+      }
+
+      tasks.withType(JavaCompile) {
+        options.with {
+          fork = true
+          forkOptions.javaHome = java8Home as File
+        }
+      }
+
+      test {
 
 Review comment:
   @Ardagan regarding first comment -- it will compile tests with java 8.  TBH I thought that approach is the first we want to take. Possible next steps would be to compile/run with java 11. On the second thought, what would you like to achieve is to check if user's code compiled with J11 would run with beam (compiled with J8)? Let me know what you think

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] iemejia edited a comment on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
iemejia edited a comment on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-599387419
 
 
   I see, your approach makes sense notice that my first comment is because we do not run examples as part of other VR tests, notice how slow the examples are in comparison with the tests, so maybe worth to split them as is already the case for Java 8.
   
   The postcommit point is arguable as you mention. In any case if Java 11 is broken on postcommit we will notice it if not quickly at least at pre release time. We can consider it by default but that means extra time for everyone so the point is if it is worth or not. I don't think we run the tests for every python version in precommit. We should probably be consistent.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] mwalenia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
mwalenia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598711199
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] pawelpasterz edited a comment on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
pawelpasterz edited a comment on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-599379680
 
 
   @iemejia 
   Hey! Thanks for comments. Regarding your first question. We wanted to simulate case when beam (compiled with J8 ) cooperates with user code that runs on J11.  This is the first step, future work will involve compiling with java 11. Regarding postcommits, that is an excellent question, my concern is that not so many people runs postcommits. Since we are aiming full java 11 support we want to know if it is working or not in advance. Summing all above -- I think that is approach we want to proceed (for the time being) 
   
   cc @Ardagan @mwalenia 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] kamilwu commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
kamilwu commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598763594
 
 
   retest this please

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] pawelpasterz commented on a change in pull request #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
pawelpasterz commented on a change in pull request #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#discussion_r392319004
 
 

 ##########
 File path: build.gradle
 ##########
 @@ -335,3 +335,27 @@ if (project.hasProperty('javaLinkageArtifactIds')) {
     }
   }
 }
+
+if (project.hasProperty('runWithJava11')) {
+  allprojects {
+    plugins.withId('java') {
+
+      java {
+        sourceCompatibility = JavaVersion.VERSION_1_8
+        targetCompatibility = JavaVersion.VERSION_1_8
+      }
+
+      tasks.withType(JavaCompile) {
+        options.with {
+          fork = true
+          forkOptions.javaHome = java8Home as File
+        }
+      }
+
+      test {
 
 Review comment:
   @Ardagan 
   Hey! I've added verification test to check bytecode and jvm.  It is launched only with `-PrunWithJava11` flag. I've already checked few jobs and did not noticed anything wrong. Let me know what do you think, thanks!

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] pawelpasterz commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
pawelpasterz commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-599379680
 
 
   @iemejia 
   Hey! Thanks for comments. Regarding your first questions. We wanted to simulate case when beam (compiled with J8 ) cooperates with user code that runs on J11.  This is the first step, future work will involve compiling with java 11. Regarding postcommits, that is an excellent question, my concern is that not so many people runs postcommits. Since we are aiming full java 11 support we want to know if it is working or not in advance. Summing all above -- I think that is approach we want to proceed (for the time being) 
   
   cc @Ardagan @mwalenia 

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] mwalenia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
mwalenia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-596475318
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] kkucharc commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
kkucharc commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-596486207
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] pawelpasterz commented on a change in pull request #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
pawelpasterz commented on a change in pull request #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#discussion_r394822688
 
 

 ##########
 File path: sdks/java/testing/test-utils/build.gradle
 ##########
 @@ -39,3 +39,14 @@ dependencies {
   testCompile library.java.hamcrest_library
   testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadowTest")
 }
+
+task verifyJavaVersion(type: Test) {
+  filter {
+    includeTestsMatching 'org.apache.beam.sdk.testutils.jvmverification.JvmVerification.verifyCodeIsCompiledWithJava8'
+    includeTestsMatching 'org.apache.beam.sdk.testutils.jvmverification.JvmVerification.verifyTestCodeIsCompiledWithJava8'
 
 Review comment:
   Thanks for input. I need to rewrite some logic then. Closing this PR for now

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] iemejia edited a comment on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
iemejia edited a comment on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-599387419
 
 
   I see, your approach makes sense notice that my first comment is because we do not run examples as part of other VR tests, notice how slow the examples are in comparison with the tests, so maybe worth to split them as is already the case for Java 8.
   
   The postcommit point is arguable as you mention, in any case if Java 11 is broken on postcommit we will notice it if not quickly at least at pre release time. We can consider it by default but that means extra time for everyone so the point is if it is worth or not. See for example we do not run the tests for every python version in precommit. We should probably be consistent.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] Ardagan commented on a change in pull request #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
Ardagan commented on a change in pull request #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#discussion_r393831160
 
 

 ##########
 File path: sdks/java/testing/test-utils/build.gradle
 ##########
 @@ -39,3 +39,14 @@ dependencies {
   testCompile library.java.hamcrest_library
   testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadowTest")
 }
+
+task verifyJavaVersion(type: Test) {
 
 Review comment:
   It is best to generalize this method since we have different options like:
   Java8 Beam, Java 8 Test, Java 8 runtime
   Java8 Beam, Java 11 Test, Java 11 runtime
   eventually we will add
   Java11 Beam, Java 8 Test, Java 8 runtime
   Java11 Beam, Java 11 Test, Java 11 runtime

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] kamilwu commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
kamilwu commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598753285
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] kamilwu commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
kamilwu commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598763322
 
 
   retest this please

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] kamilwu commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
kamilwu commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598756537
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] iemejia edited a comment on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
iemejia edited a comment on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598812820
 
 
   Is running the examples with Java 11 supposed to be part of this too? This is separate for the Java 8 case. Saying this because that part (the examples) is taking longer time than the proper tests itself in the gradle link https://scans.gradle.com/s/2fxsbki3ecfes/tests/by-project?toggled=Wzhd

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] pawelpasterz commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
pawelpasterz commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-596545210
 
 
   @kennknowles @lukecwik @Ardagan @mwalenia 
   Could you take a look and review? This is other solution I've found to solve #10878 .
   We have tested it with https://builds.apache.org/job/beam_PreCommit_JavaPortabilityApiJava11_Phrase/7/
   
   Let me know what do you think, thanks!

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] iemejia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
iemejia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-598813195
 
 
   Also given that we are testing already the portability API and that our base support is Java 8 shouldn't the Java 11 case be a postcommit?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] mwalenia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
mwalenia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-596462741
 
 
   Run JavaPortabilityApiJava11 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


With regards,
Apache Git Services

[GitHub] [beam] Ardagan commented on a change in pull request #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
Ardagan commented on a change in pull request #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#discussion_r391112088
 
 

 ##########
 File path: build.gradle
 ##########
 @@ -335,3 +335,27 @@ if (project.hasProperty('javaLinkageArtifactIds')) {
     }
   }
 }
+
+if (project.hasProperty('runWithJava11')) {
+  allprojects {
+    plugins.withId('java') {
+
+      java {
+        sourceCompatibility = JavaVersion.VERSION_1_8
+        targetCompatibility = JavaVersion.VERSION_1_8
+      }
+
+      tasks.withType(JavaCompile) {
+        options.with {
+          fork = true
+          forkOptions.javaHome = java8Home as File
+        }
+      }
+
+      test {
 
 Review comment:
   I'd also suggest to add test that validates runtime/compile time java version. This will make reviews much easier.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] mwalenia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
mwalenia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-596504336
 
 
   run seed job

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] iemejia commented on issue #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
iemejia commented on issue #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#issuecomment-599387419
 
 
   I see, your approach makes sense notice that my first comment is because we do not run tests as part of other VR tests, notice how slow the examples are in comparison with the tests, so maybe worth to split them as is already the case for Java 8.
   
   The postcommit point is arguable as you mention, in any case if Java 11 is broken on postcommit we will notice it if not quickly at least at pre release time. We can consider it by default but that means extra time for everyone so the point is if it is worth or not. See for example we do not run the tests for every python version in precommit. We should probably be consistent.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] pawelpasterz closed pull request #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
pawelpasterz closed pull request #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [beam] Ardagan commented on a change in pull request #11076: Run precommit portability on java 11

Posted by GitBox <gi...@apache.org>.
Ardagan commented on a change in pull request #11076: Run precommit portability on java 11
URL: https://github.com/apache/beam/pull/11076#discussion_r393832528
 
 

 ##########
 File path: sdks/java/testing/test-utils/build.gradle
 ##########
 @@ -39,3 +39,14 @@ dependencies {
   testCompile library.java.hamcrest_library
   testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadowTest")
 }
+
+task verifyJavaVersion(type: Test) {
+  filter {
+    includeTestsMatching 'org.apache.beam.sdk.testutils.jvmverification.JvmVerification.verifyCodeIsCompiledWithJava8'
+    includeTestsMatching 'org.apache.beam.sdk.testutils.jvmverification.JvmVerification.verifyTestCodeIsCompiledWithJava8'
 
 Review comment:
   We should add test that compiles beam with java8 and tests with java11. This use case is more important. I don't think it is common to compile your pipeline with Java8 and then run with Java11.

----------------------------------------------------------------
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


With regards,
Apache Git Services