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