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 2022/04/04 23:28:52 UTC
[GitHub] [beam] guillaumecle opened a new pull request, #17274: BEAM-13468 allow non-lts jvm version
guillaumecle opened a new pull request, #17274:
URL: https://github.com/apache/beam/pull/17274
It possible to run beam on a non-lts jvm: either using the direct runner or uploading java code compiled in a supported version to the remote runner.
This pull request, stop throwing Exception when the current jvm is a non-lts, instead it's falling back to java 11 for the runner version.
I was able to successfully run a java 11 dataflow job from a java 15 jvm.
I think this will also fix BEAM-12408 (Direct Runner cannot be run on non Java LTS versions)
R: @kileys
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
kileys commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1239798191
Sorry for the delay
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1190846761
Yes I have successfully tested the direct runner on java 15
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
kileys commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r925947724
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -112,6 +116,20 @@ public String specification() {
}
public static JavaVersion forSpecification(String specification) {
+ for (JavaVersion ver : JavaVersion.values()) {
+ if (ver.specification.equals(specification)) {
+ return ver;
+ }
+ }
+ JavaVersion fallback = java11;
Review Comment:
Instead of always falling back to 11, could we find the nearest LTS version to fall back to?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r842216206
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -117,8 +121,9 @@ public static JavaVersion forSpecification(String specification) {
return ver;
}
}
- throw new UnsupportedOperationException(
- String.format("unsupported Java version: %s", specification));
+ JavaVersion fallback = java11;
+ LOG.warn("unsupported Java version: {}, falling back to: {}", specification, fallback.specification);
Review Comment:
This logger might be a bit too chatty, I can reduce the log level if needed.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1088116207
Can one of the admins verify this patch?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1088116225
Can one of the admins verify this patch?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1090454835
R: @kileys
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1121635787
This PR is ready for any additional reviews/comments.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
aaltay commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1112496170
What is the next step on this PR?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1088116276
Can one of the admins verify this patch?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r927975021
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -112,6 +116,20 @@ public String specification() {
}
public static JavaVersion forSpecification(String specification) {
+ for (JavaVersion ver : JavaVersion.values()) {
+ if (ver.specification.equals(specification)) {
+ return ver;
+ }
+ }
+ JavaVersion fallback = java11;
Review Comment:
I can see 2 strategies:
1. find the absolute nearest. lower or higher
2. find the nearest higher. if you have code compile in java 13 you can hope to have it working on java 17 but certainly not on java 11
It's still possible to have compiled your code with 11 but run it on java 13 jvm. this is why solution 1 can work and I'm going with it 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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r853508942
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -117,8 +121,12 @@ public static JavaVersion forSpecification(String specification) {
return ver;
}
}
- throw new UnsupportedOperationException(
- String.format("unsupported Java version: %s", specification));
+ JavaVersion fallback = java11;
Review Comment:
Sure, I will add that.
Do you happen to already know some places where we would use the stricter version ?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1088116211
Can one of the admins verify this patch?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] emilymye commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
emilymye commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r849046219
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -117,8 +121,12 @@ public static JavaVersion forSpecification(String specification) {
return ver;
}
}
- throw new UnsupportedOperationException(
- String.format("unsupported Java version: %s", specification));
+ JavaVersion fallback = java11;
Review Comment:
Could we have two methods here, where one is "fromSpecificationOrDefault" or similar? There are some cases where we probably want to have a strict check (i.e. expecting translatation from one of the existing enums, looking for a specific supported Docker container version where defaulting can lead to confusing behavior)
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -117,8 +121,12 @@ public static JavaVersion forSpecification(String specification) {
return ver;
}
}
- throw new UnsupportedOperationException(
- String.format("unsupported Java version: %s", specification));
+ JavaVersion fallback = java11;
Review Comment:
Could we have two methods here, where one is "forSpecificationOrDefault" or similar? There are some cases where we probably want to have a strict check (i.e. expecting translatation from one of the existing enums, looking for a specific supported Docker container version where defaulting can lead to confusing behavior)
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1201759716
This PR is ready for review and approval.
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1228711929
@kileys Could you take another look at this pr?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1088116208
Can one of the admins verify this patch?
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] emilymye commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
emilymye commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r849046219
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -117,8 +121,12 @@ public static JavaVersion forSpecification(String specification) {
return ver;
}
}
- throw new UnsupportedOperationException(
- String.format("unsupported Java version: %s", specification));
+ JavaVersion fallback = java11;
Review Comment:
Could we have two methods here, where one is "getJavaVersionOrDefault" or similar? There are some cases where we probably want to have a strict check (i.e. expecting translatation from one of the existing enums, looking for a specific supported Docker container version where defaulting can lead to confusing behavior)
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys commented on pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
kileys commented on PR #17274:
URL: https://github.com/apache/beam/pull/17274#issuecomment-1190632366
Thanks for the PR. Were you able to test the direct runner on a non-LTS version 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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r927992928
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -112,6 +116,20 @@ public String specification() {
}
public static JavaVersion forSpecification(String specification) {
+ for (JavaVersion ver : JavaVersion.values()) {
+ if (ver.specification.equals(specification)) {
+ return ver;
+ }
+ }
+ JavaVersion fallback = java11;
Review Comment:
one last question is what to do for the middle point, for now I using the higher version (e.g.: use 17 when invoked from 14)
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r926115562
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -112,6 +116,20 @@ public String specification() {
}
public static JavaVersion forSpecification(String specification) {
+ for (JavaVersion ver : JavaVersion.values()) {
+ if (ver.specification.equals(specification)) {
+ return ver;
+ }
+ }
+ JavaVersion fallback = java11;
Review Comment:
Sure, working on this right 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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kileys merged pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
kileys merged PR #17274:
URL: https://github.com/apache/beam/pull/17274
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r862224385
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -117,8 +121,12 @@ public static JavaVersion forSpecification(String specification) {
return ver;
}
}
- throw new UnsupportedOperationException(
- String.format("unsupported Java version: %s", specification));
+ JavaVersion fallback = java11;
Review Comment:
I added a strict method `forSpecificationStrict`.
the java version ends up being used in
* `JAVA_SDK_HARNESS_CONTAINER_URL` for which I wan to use the lenient version for now
* `DataflowRunner.fromOptions` for which I wan to use the lenient version
* `DataflowRunner.getDefaultContainerImageNameForJob` for which I wan to use the lenient version
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guillaumecle commented on a diff in pull request #17274: BEAM-13468 allow non-lts jvm version
Posted by GitBox <gi...@apache.org>.
guillaumecle commented on code in PR #17274:
URL: https://github.com/apache/beam/pull/17274#discussion_r862224385
##########
runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/Environments.java:
##########
@@ -117,8 +121,12 @@ public static JavaVersion forSpecification(String specification) {
return ver;
}
}
- throw new UnsupportedOperationException(
- String.format("unsupported Java version: %s", specification));
+ JavaVersion fallback = java11;
Review Comment:
I added a strict method `forSpecificationStrict`.
the java version ends up being used in
* `JAVA_SDK_HARNESS_CONTAINER_URL` for which I want to use the lenient version for now
* `DataflowRunner.fromOptions` for which I want to use the lenient version
* `DataflowRunner.getDefaultContainerImageNameForJob` for which I want to use the lenient version
--
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.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org