You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/10/31 18:41:40 UTC
[GitHub] [flink-statefun] galenwarren opened a new pull request, #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
galenwarren opened a new pull request, #319:
URL: https://github.com/apache/flink-statefun/pull/319
Upgrade supported Flink version to 1.15.2.
**Note**: The e2e tests are failing, with this error:
```
4:37:02,233 ERROR org.apache.flink.statefun.e2e.smoke.SmokeRunner - Exception in thread "main" java.lang.ClassCastException: java.lang.String cannot be cast to java.util.List
14:37:02,233 ERROR org.apache.flink.statefun.e2e.smoke.SmokeRunner - at org.apache.flink.statefun.flink.launcher.StatefulFunctionsClusterEntryPoint.addStatefulFunctionsConfiguration(StatefulFunctionsClusterEntryPoint.java:134)
14:37:02,233 ERROR org.apache.flink.statefun.e2e.smoke.SmokeRunner - at org.apache.flink.statefun.flink.launcher.StatefulFunctionsClusterEntryPoint.main(StatefulFunctionsClusterEntryPoint.java:88)
```
That line is reading `CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL`, which was changed from a `String` to a `List<String>` in 1.15. The e2e tests depend on a published Statefun Docker image, the latest version of which is for Statefun 3.2 and which references Flink 1.14.x. So, I think a new Statefun image would need to be published to allow the e2e tests to work. I'm not sure how to do that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] galenwarren commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
galenwarren commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1297841266
OK, I got it working. I sent five messages and got what look like the expected replies:
```
(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X PUT -H "Content-Type: application/vnd.greeter.types/UserLogin" -d '{"user_id": "1", "user_name": "Joe", "login_type": "WEB"}' localhost:8090/greeter.fns/user/1
(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X GET localhost:8091/greetings
Welcome Joe!(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X PUT -H "Content-Type: application/vnd.greeter.types/UserLogin" -d '{"user_id": "1", "user_name": "Joe", "login_type": "WEB"}' localhost:8090/greeter.fns/user/1
(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X GET localhost:8091/greetings
Nice to see you again Joe.(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X PUT -H "Content-Type: application/vnd.greeter.types/UserLogin" -d '{"user_id": "1", "user_name": "Joe", "login_type": "WEB"}' localhost:8090/greeter.fns/user/1
(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X GET localhost:8091/greetings
Third time is a charm Joe!(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X PUT -H "Content-Type: application/vnd.greeter.types/UserLogin" -d '{"user_id": "1", "user_name": "Joe", "login_type": "WEB"}' localhost:8090/greeter.fns/user/1
(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X GET localhost:8091/greetings
Nice to see you for the 4th time, Joe! It has been 93593 milliseconds since we last saw you.(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X PUT -H "Content-Type: application/vnd.greeter.types/UserLogin" -d '{"user_id": "1", "user_name": "Joe", "login_type": "WEB"}' localhost:8090/greeter.fns/user/1
(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ curl -X GET localhost:8091/greetings
Nice to see you for the 5th time, Joe! It has been 7917 milliseconds since we last saw you.(base) galen@galen-ubuntu-20:~/projects/git/coachclientconnect$ [<35;39;50M[<35;24;40M[<35;13;35M```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] galenwarren commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
galenwarren commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1297555582
@tzulitai Yes, I will run the playground examples and report back.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] galenwarren commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
galenwarren commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1297546217
Thanks! Yes, you're right, and I didn't figure that out until after the first commit. I've updated the Flink base image tag and the e2e tests ran successfully locally. They're running now in CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] tzulitai commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
tzulitai commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1297542400
Thanks for opening a PR @galenwarren!
The e2e tests would first build a new local image (which would reference Flink 1.15.2), and then use that to run the e2e tests.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] tzulitai closed pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
tzulitai closed pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
URL: https://github.com/apache/flink-statefun/pull/319
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] tzulitai commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
tzulitai commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1297554015
@galenwarren could you try that out and report back? If all is good, then +1 to merge this.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] galenwarren commented on a diff in pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
galenwarren commented on code in PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#discussion_r1009848504
##########
statefun-flink/statefun-flink-core/src/main/java/org/apache/flink/statefun/flink/core/StatefulFunctionsConfigValidator.java:
##########
@@ -57,10 +57,12 @@ private static void validateParentFirstClassloaderPatterns(Configuration configu
}
private static Set<String> parentFirstClassloaderPatterns(Configuration configuration) {
- final String[] split =
- configuration.get(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL).split(";");
- final Set<String> parentFirstClassloaderPatterns = new HashSet<>(split.length);
- for (String s : split) {
+ final String[] patterns =
+ configuration
+ .get(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL)
+ .toArray(new String[0]);
Review Comment:
Yes, good point. I'll update that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] galenwarren commented on a diff in pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
galenwarren commented on code in PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#discussion_r1009844967
##########
README.md:
##########
@@ -155,7 +155,7 @@ This section contains information for building this project.
2. Build Stateful Functions Docker image: This step requires that you've already compiled artifacts from the source code.
```
- $ ./tools/docker/build-distribution.sh
+ $ ./tools/docker/build-stateful-functions.sh
Review Comment:
That's actually the name of the script: `./tools/docker/build-stateful-functions.sh`. The README in the docker folder has the right name but the top-level README had `./tools/docker/build-distribution.sh`.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] galenwarren commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
galenwarren commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1297715423
@tzulitai I'm having a couple issues with the playground tests.
First issue, which I've worked around (I think!) is that the build of the playground projects occurs in a Docker container, so it doesn't have access to my local Maven repository. I think I can work around this by building the project outside of a container and temporarily modifying the Dockerfile to skip the build step and to copy the jar file in from the host.
However, the second issue I'm running into is that I think I need to have built `apache/flink-statefun-playground:3.3.0-1.0` somewhere along the way, so that it can be accessed when `docker-compose up` is called. Is that built somewhere during the flink-statefun build process?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] tzulitai commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
tzulitai commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1297819016
@galenwarren right, its actually a bit more complicated then what I described indeed 😅
You'll have to edit the Dockerfile to manually copy the StateFun Java SDK jar in, instead of letting it pull from Maven.
> However, the second issue I'm running into is that I think I need to have built apache/flink-statefun-playground:3.3.0-1.0
Ah, the use of the `apache/flink-statefun-playground` image for the playground examples is a more recent development I'm not familiar with. I think @tillrohrmann / @igalshilman would be able to provide info here?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] galenwarren commented on a diff in pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
galenwarren commented on code in PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#discussion_r1009848181
##########
pom.xml:
##########
@@ -79,11 +79,12 @@ under the License.
<protobuf.version>3.7.1</protobuf.version>
<unixsocket.version>2.3.2</unixsocket.version>
<protoc-jar-maven-plugin.version>3.11.1</protoc-jar-maven-plugin.version>
- <flink.version>1.14.3</flink.version>
+ <flink.version>1.15.2</flink.version>
<scala.binary.version>2.12</scala.binary.version>
<scala.version>2.12.7</scala.version>
<lz4-java.version>1.8.0</lz4-java.version>
- <flink-shaded-jackson.version>2.12.4-14.0</flink-shaded-jackson.version>
+ <flink-shaded-jackson.version>2.12.4-15.0</flink-shaded-jackson.version>
+ <slf4j-log4j12.version>1.7.32</slf4j-log4j12.version>
Review Comment:
`org.slf4j:slf4j-log4j12` is referenced in two subprojects: `statefun-smoke-e2e-common` and `statefun-flink-distribution`, and I needed to increase the version to resolve a dependency convergence issue. So I thought I'd pull the version out to the parent pom. But I'm happy to do something else if you'd prefer.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] tzulitai commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
tzulitai commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1297552879
What I usually like to do before merging a Flink upgrade in StateFun is to manually test out the upgrade using some of our playground examples. If that works + CI gives green light here, then we should be good to go.
This is the playground repo: https://github.com/apache/flink-statefun-playground/tree/dev
Since you've already locally build the new image, all you'd have to do is go into one of the examples, say `java/greeter`, and then update the StateFun image version in the docker-compose file and pom.xml files. Then, manually run the example and observe that everything is as expected.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] tzulitai commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
tzulitai commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1298826258
Merged to `master`: b4a85ef031e539e7c2580a796246e2896d533d8f
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] tzulitai commented on pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
tzulitai commented on PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#issuecomment-1298804771
Thanks @galenwarren for the PR and @FilKarnicki for testing this as well.
+1 to merge, merging ...
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] tzulitai commented on a diff in pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
tzulitai commented on code in PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#discussion_r1009778227
##########
statefun-flink/statefun-flink-core/src/main/java/org/apache/flink/statefun/flink/core/StatefulFunctionsConfigValidator.java:
##########
@@ -57,10 +57,12 @@ private static void validateParentFirstClassloaderPatterns(Configuration configu
}
private static Set<String> parentFirstClassloaderPatterns(Configuration configuration) {
- final String[] split =
- configuration.get(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL).split(";");
- final Set<String> parentFirstClassloaderPatterns = new HashSet<>(split.length);
- for (String s : split) {
+ final String[] patterns =
+ configuration
+ .get(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL)
+ .toArray(new String[0]);
Review Comment:
Probably don't really need `patterns` to be a `String[]`. Fine to just let it be a `List<String>` to keep things simple.
##########
pom.xml:
##########
@@ -79,11 +79,12 @@ under the License.
<protobuf.version>3.7.1</protobuf.version>
<unixsocket.version>2.3.2</unixsocket.version>
<protoc-jar-maven-plugin.version>3.11.1</protoc-jar-maven-plugin.version>
- <flink.version>1.14.3</flink.version>
+ <flink.version>1.15.2</flink.version>
<scala.binary.version>2.12</scala.binary.version>
<scala.version>2.12.7</scala.version>
<lz4-java.version>1.8.0</lz4-java.version>
- <flink-shaded-jackson.version>2.12.4-14.0</flink-shaded-jackson.version>
+ <flink-shaded-jackson.version>2.12.4-15.0</flink-shaded-jackson.version>
+ <slf4j-log4j12.version>1.7.32</slf4j-log4j12.version>
Review Comment:
Why is this needed now?
##########
README.md:
##########
@@ -155,7 +155,7 @@ This section contains information for building this project.
2. Build Stateful Functions Docker image: This step requires that you've already compiled artifacts from the source code.
```
- $ ./tools/docker/build-distribution.sh
+ $ ./tools/docker/build-stateful-functions.sh
Review Comment:
was this done by accident?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-statefun] galenwarren commented on a diff in pull request #319: [FLINK-29814][statefun] Change supported Flink version to 1.15.2
Posted by GitBox <gi...@apache.org>.
galenwarren commented on code in PR #319:
URL: https://github.com/apache/flink-statefun/pull/319#discussion_r1009894290
##########
statefun-flink/statefun-flink-core/src/main/java/org/apache/flink/statefun/flink/core/StatefulFunctionsConfigValidator.java:
##########
@@ -57,10 +57,12 @@ private static void validateParentFirstClassloaderPatterns(Configuration configu
}
private static Set<String> parentFirstClassloaderPatterns(Configuration configuration) {
- final String[] split =
- configuration.get(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL).split(";");
- final Set<String> parentFirstClassloaderPatterns = new HashSet<>(split.length);
- for (String s : split) {
+ final String[] patterns =
+ configuration
+ .get(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL)
+ .toArray(new String[0]);
Review Comment:
Fixed in https://github.com/apache/flink-statefun/pull/319/commits/d2508875511442734ac9296a96354d6f6a0c965a.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org