-
Notifications
You must be signed in to change notification settings - Fork 20
Upgrade Spring Boot and Java #194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
georgweiss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the Olog service we're about to switch to:
- Java 25 (to benefit from language features like lightweight threads)
- Spring Boot 3.5.7
- Jackson 3.0.1
Would this be viable for Channel Finder aswell?
|
@georgweiss we specified these versions for now, but we can create follow up for java 25 + highest spring boot 3, and then follow that with spring boot 4 |
anderslindho
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@jacomago do we wanna set a requirement for the sonar lints - whether they must be addressed?
and do we want to do something about the tests?
| * JDK 17 | ||
| * JDK 21 | ||
| * Maven (via package manager or via the wrapper `./mvnw`) (version specified | ||
| in [the wrapper properties](./.mvn/wrapper/maven-wrapper.properties)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you need to update also this? (maven wrapper)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I upgrade it to 3.9+?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep it out of scope - please create a backlog task re maven version
|
@anderslindho, since the Channel Finder service is an integral part of the Phoebus stack, it might be seen as a bit confusing to not run on the same base dependencies. |
|
We do not have a precise time plan, as we are agile. We can bump this in prio based on value, if you think this can cause confusion for maintainers. It would, however, be categorised as a maintenance change, to be prioritised lower than support and dev. Regardless, we often manage to squeeze in smaller maintenance updates in between support and dev work. Do you have any targets for the rest of the services? |
compose.yml
Outdated
|
|
||
| elasticsearch: | ||
| image: docker.elastic.co/elasticsearch/elasticsearch:8.11.2 | ||
| image: docker.elastic.co/elasticsearch/elasticsearch:8.18.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we update elastic in a different MR? It changes the requirements for running.
| } | ||
|
|
||
| @Bean | ||
| public PasswordEncoder encoder() { | ||
| return new BCryptPasswordEncoder(); | ||
| } | ||
|
|
||
| private static class EmbeddedLdapCondition implements Condition { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems mixed up with demo auth and embedded ldap. They should be separate beans.
| @@ -30,7 +30,7 @@ | |||
| class ChannelProcessorManagerIT { | |||
|
|
|||
| protected static final String AUTHORIZATION = | |||
| "Basic " + Base64Utils.encodeToString("admin:adminPass".getBytes()); | |||
| "Basic " + Base64.getEncoder().encodeToString("admin:adminPass".getBytes()); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting failures on this test. I'm guessing because of the mix of embeddedldap and demo auth.
|
@anderslindho, Olog will be merged this week. For the other services I expect these to be migrated early next year. |
ac70a12 to
f1a0920
Compare
Upgrade the Spring Boot version to 3.x.x and the Java version to 21. Update dependencies for compatibility, and align the code with these upgrades, especially in WebSecurityConfig.
Update Docker related files to use Java 21 and elasticsearch 8.18.0
Update readme to reference the correct versions.
Uplift Java version in teh Github workflow files
It have to be done later in a separate pull request.
47b11e7 to
aa26c17
Compare
|


Changes: