java-sdk
java-sdk copied to clipboard
dapr
Bump supported Spring Boot baseline to version 3.2.x
Description
Spring Boot < 3.2.x reached end of life and it's not maintained anymore. This PR bumps the minimum supported version to 3.2.x, supporting both the 3.2.x and 3.3.x release trains.
- Bump dependencies in sdk-springboot module from Spring Boot 2.x to 3.2.6
- Update examples module to use new Spring Boot support with Java 17 baseline
- Fix wrong sdkman and maven wrapper setup that failed the local setup
- Update GHA workflow to stop testing for Spring Boot versions < 3.2.x
Issue reference
Fixes gh-1039
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:
- [x] Code compiles correctly
- [x] Created/updated tests
- [x] Extended the documentation
@artursouza I just checked this PR.. and this is what we need to do to upgrade to spring 3.2.x .. We will start refactoring the spring boot support, trying not to break existing users.
Looks like CI is failing the license/synk check, but I can't view it. I'm fine with proceeding with this change given if there are vulnerabilities in Spring Boot < 3.2.x it won't be fixed and users have had some time to upgrade by now.
@cicoyle thanks for the review. I can't see the Snyk report either. @salaboy do you have access? I'm not sure which dependency is triggering the failure.
@cicoyle when you have time can you please validate this one? now that the HTTP client is removed .. we need to upgrade the spring boot version.
@cicoyle thanks for the review. I can't see the Snyk report either. @salaboy do you have access? I'm not sure which dependency is triggering the failure.
Let me ask for permission
@ThomasVitale I am working out on the licence stuff.. but it seems that dapr-sdk-tests is still trying to get Could not find artifact io.dapr:dapr-sdk-springboot3:jar:1.12.0-SNAPSHOT which might be because you forgot to update that dependency. Here: https://github.com/dapr/java-sdk/blob/master/sdk-tests/pom.xml#L132
@ThomasVitale I am working out on the licence stuff.. but it seems that
dapr-sdk-testsis still trying to getCould not find artifact io.dapr:dapr-sdk-springboot3:jar:1.12.0-SNAPSHOTwhich might be because you forgot to update that dependency. Here: https://github.com/dapr/java-sdk/blob/master/sdk-tests/pom.xml#L132
I fixed it in FOSSA. Ignore Snyk's report on license, I disabled that going forward. Snyk is used for vulnerability reports only.
@salaboy thanks, I had missed that sdk-tests was excluded from the multi-module Maven project, so the tests didn't actually run as part of the local workflow. I tried to set them up locally based on the pipeline content, but I didn't manage to do it fully, I'm afraid. I pushed a change to update the dependencies in dk-tests, and remove sdk-springboot3. Can you re-run the PR pipeline?
@salaboy thanks, I had missed that sdk-tests was excluded from the multi-module Maven project, so the tests didn't actually run as part of the local workflow. I tried to set them up locally based on the pipeline content, but I didn't manage to do it fully, I'm afraid. I pushed a change to update the dependencies in dk-tests, and remove sdk-springboot3. Can you re-run the PR pipeline?
This is a breaking change but it might make sense for the Java community to simply move away from Springboot 2.
Yes we made the decision to move forward to spring 3.2.x .. let’s unblock this if the PR is green
-
Blog: http://salaboy.com http://salaboy.wordpress.com
-
Github user: http://github.com/salaboy
-
Twitter: http://twitter.com/salaboy
-
Mauricio "Salaboy" Salatino -
On Wed, 26 Jun 2024 at 19:12, Artur Souza @.***> wrote:
@salaboy https://github.com/salaboy thanks, I had missed that sdk-tests was excluded from the multi-module Maven project, so the tests didn't actually run as part of the local workflow. I tried to set them up locally based on the pipeline content, but I didn't manage to do it fully, I'm afraid. I pushed a change to update the dependencies in dk-tests, and remove sdk-springboot3. Can you re-run the PR pipeline?
This is a breaking change but it might make sense for the Java community to simply move away from Springboot 2.
— Reply to this email directly, view it on GitHub https://github.com/dapr/java-sdk/pull/1050#issuecomment-2192351213, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACCMXV6EOICF7PRK6KXLZTZJMABBAVCNFSM6AAAAABJIVLXTGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJSGM2TCMRRGM . You are receiving this because you were mentioned.Message ID: @.***>
@salaboy and @ThomasVitale I have spent some time looking at the build failures. According to this document: https://github.com/spring-projects/spring-framework/wiki/Upgrading-to-Spring-Framework-6.x#parameter-name-retention things have changed in Spring Framework 6.x when it comes to parameter name resolution. We have to either use Java Compiler with -parameters flag or ensure that @PathVariable and @RequestPathParam are explicitly named. Here is the IT related file that has to be adjusted: https://github.com/dapr/java-sdk/blob/342f8045830cc96d308df2670a06b8d65f8cf19b/sdk-tests/src/test/java/io/dapr/it/methodinvoke/http/MethodInvokeController.java#L21.
I have run an experiment using explicit naming that can be found here: https://github.com/artur-ciocanu/java-sdk/actions/runs/9696067245/job/26757186160. The GitHub Action succeeded, so we can at least try to fix these issues in the near term.
I am still looking into other build failures. I'll keep you posted.
CC: @artursouza @cicoyle
@salaboy and @ThomasVitale I have spent a few hours figuring out why the tests are failing. Here are my findings:
- "parameter name resolution" - the are changes in SB 2.x and SB 3.x it is due to Spring Framework 6.x changes. The fixes are trivial I have suggested them in the previous comment.
- OpenTelemetry version mismatch - it seems that as part of SB 3.x upgrade we get a new version of Zipkin reporter and this conflicts with OTEL version that is currently used in
sdk-tests. Here is the error that I managed to reproduce:
Error: Errors:
Error: TracingIT.testInvoke:60 » NoSuchMethod 'zipkin2.Call zipkin2.reporter.Sender.sendSpans(java.util.List)'
Error: TracingIT.testInvoke:58 » NoSuchMethod 'zipkin2.Call zipkin2.reporter.Sender.sendSpans(java.util.List)'
My suggestion for the second issue would be to upgrade OTEL to latest library, but this requires some significant changes in sdk-tests since currently it uses 0.14.0 while the latest is 1.39.0.
@artursouza and @cicoyle what are your thoughts.
@artur-ciocanu I wonder if we can rely on the OTEL stuff that comes with Spring Boot 3.x instead of adding our own dependency
@artur-ciocanu I wonder if we can rely on the OTEL stuff that comes with Spring Boot 3.x instead of adding our own dependency
@salaboy I was thinking about it ... I guess for Spring Boot based IT tests we can, but I would have to defer to @artursouza and others, since I don't know why we had to use OTEL in its "raw" form. I am assuming because of GRPC tests, but I am not sure.
In any case if we upgrade OTEL the IT tests have to be adjusted.
@cicoyle @artursouza or @ThomasVitale could you please try to merge the "master" and see if we still have any issues with Spring Boot upgrade.
@artur-ciocanu I have rebased this PR on the current "master" and pushed the changes
@artursouza and @cicoyle it seems that we need maintainers approval. The build workflows are blocked right now.
Whenever you have a chance, could you please help? Thank you.
@ThomasVitale we are almost there 😀. It seems that there is still one IT test that is failing. It is related to SDK resiliency. I will take a look on Monday and see if I can reproduce it and come up with a fix.
@ThomasVitale and @salaboy I have opened #1070 to try to fix the SDK resiliency IT failure. Let's wait until it is merged and see if it will fix the issue.
@ThomasVitale the SDK resiliency IT fix PR has been merged. Could you please merge "master" and see if things are looking good.
@artursouza or @cicoyle it seems that we need maintainers approval to kick off the GitHub workflows. Could you please approve and if everything looks merge the PR.
Thank you! 🙇♀️
Build is still failing. I have no problem merging this once validation passes.
Let me check tomorrow one more time to see what might be the issue. Thanks for checking @artursouza 🙇
Thanks! I would like to help debug the integration tests, but I didn't manage to run them locally. I have suggested an improvement for the docs on how to establish a local setup for running the sdk-tests project. https://github.com/dapr/java-sdk/issues/1075