Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update spring-boot to 2.7.9 #1310

Merged
merged 24 commits into from
Mar 16, 2023
Merged

Update spring-boot to 2.7.9 #1310

merged 24 commits into from
Mar 16, 2023

Conversation

MichaelsJP
Copy link
Member

@MichaelsJP MichaelsJP commented Mar 6, 2023 •
edited

Details

Pull Request Checklist

  • 1. I have rebased the latest version of the master branch into my feature branch and all conflicts
    have been resolved.
  • 2. I have added information about the change/addition to functionality to the CHANGELOG.md file under the
    [Unreleased] heading.
  • 4. I have removed unnecessary commented out code, imports and System.out.println statements.
  • 8. I have built graphs with my code of the Heidelberg.osm.gz file and run the api-tests with all test passing
  • 12. I have written in the Pull Request information about the changes made including their intended usage
    and why the change was needed.

Things to check before merge:

  • check the base api url consistency (/ors/v2 vs /v2).
    • the context-path setting enforces /ors also in production when run with maven. This is a breaking change and should be avoided. The easier way is to remove the /ors completely. That means local dev setups won't have this path.
  • change run_api_tests.sh to curl for v2/status instead of ors.run
  • check for correct config file configurations (run path of spring -> built-in tomcat is different from tomcat7 -> spring )
  • Add dev setup documentation for intelllij
  • Fix docker setup

Fixes #1151
Closes #1026
closes #1285
Partially touches #1025

Information about the changes

  • Key functionality added: Upgrade to spring-boot 2.7.9. Partially, the code wasn't properly using java11 and had to be fixed.
  • Reason for change: The current spring-boot version is outdated and not maintained anymore.

Required changes to ors config (if applicable)

  • The new spring-boot update requires us to use the built-in tomcat runner. That runner has a different run path than the original tomcat7 servlet container. The testing config needs to be adjusted for this.

@MichaelsJP MichaelsJP changed the title chore: Fix spring conversion incompatible comments Spring update to 2.7.9 Mar 6, 2023
@takb takb added this to To do in ors general Mar 6, 2023
@MichaelsJP MichaelsJP changed the title Spring update to 2.7.9 Update spring-boot to 2.7.9 Mar 6, 2023
@MichaelsJP
Copy link
Member Author

Supersedes the old PR #1285 to spring 2.7.8

Closes #1285

@MichaelsJP MichaelsJP force-pushed the spring-update-to-2.7.9 branch 2 times, most recently from 04f99ff to b6ab70a Compare March 6, 2023 19:13
@takb takb mentioned this pull request Mar 7, 2023
Closed
@takb takb moved this from To do to Review in ors general Mar 7, 2023
@MichaelsJP MichaelsJP force-pushed the spring-update-to-2.7.9 branch 3 times, most recently from 69dbde3 to 9307c92 Compare March 8, 2023 09:42
@MichaelsJP MichaelsJP moved this from Review to In progress in ors general Mar 8, 2023
@MichaelsJP MichaelsJP force-pushed the spring-update-to-2.7.9 branch 8 times, most recently from f94c2a0 to 952bf98 Compare March 8, 2023 12:27
@MichaelsJP MichaelsJP requested a review from takb March 8, 2023 17:14
@MichaelsJP MichaelsJP self-assigned this Mar 9, 2023
@MichaelsJP MichaelsJP marked this pull request as ready for review March 9, 2023 09:53
jlink
jlink approved these changes Mar 9, 2023
View reviewed changes
Copy link
Contributor

@jlink jlink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as far as I'm competent which is only a little

@MichaelsJP MichaelsJP marked this pull request as draft March 9, 2023 16:05
@MichaelsJP MichaelsJP added this to the Release 7.2.0 milestone Mar 9, 2023
@MichaelsJP MichaelsJP closed this Mar 13, 2023
@MichaelsJP MichaelsJP reopened this Mar 13, 2023
ors general automation moved this from In progress to Review Mar 13, 2023
jlink
jlink approved these changes Mar 15, 2023
View reviewed changes
Copy link
Contributor

@jlink jlink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Complex running of ITs can be handled in other PR.

openrouteservice/src/main/java/org/heigit/ors/api/converters/APIRequestProfileConverter.java Outdated Show resolved Hide resolved
openrouteservice/src/main/resources/application.properties Show resolved Hide resolved
@MichaelsJP MichaelsJP force-pushed the spring-update-to-2.7.9 branch 5 times, most recently from 72d6c52 to 39ef24b Compare March 15, 2023 15:59
@MichaelsJP
fix(api): add /ors to the context-path
3a72612 
If the context-path is not defined with /ors it will fallback to /.
@MichaelsJP
test(debug): Temporal change to show the running path
31cf027
@MichaelsJP
test(api-tests): run tests based on health status code
de1943d 
Before the api tests were waiting for 'ors.run' to be created at some place. Now it looks at the /v2/health api endpoints and checks for http 200. This indicates the graphs are build and the system is ready. Timeout is set to 4 minutes
@MichaelsJP
build(spring-boot): remove fork setting
8d787d6 
The fork setting set to false mixes up run paths but allows for local debug session. When set to true it disables the ability for a local debug session but has the expected run paths. To have local debug sessions one can use  -Dspring-boot.run.fork=false to disable forking when needed. This is only possible when the fork settings is not set at all in the pom.
@MichaelsJP
chore(backend): remove annotation
ce67bfb 
intellij annotation was set by accident.
@MichaelsJP
docs(documentation): correct the java version to 11.
e05a49c
@MichaelsJP
build(spring-boot): remove provided from spring-boot-starter-actuator
4adc9b4 
spring-boot-starter-actuator is needed at runtime.
@MichaelsJP
test(api-tests): fix the api-test url path
a681237
@MichaelsJP
docs(documentation): Adjust the spring-boot:run examples
484b8c4
@MichaelsJP MichaelsJP marked this pull request as ready for review March 16, 2023 09:27
@MichaelsJP MichaelsJP marked this pull request as draft March 16, 2023 09:27
takb
takb approved these changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@takb takb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MichaelsJP MichaelsJP marked this pull request as ready for review March 16, 2023 11:15
@MichaelsJP MichaelsJP merged commit e37f104 into master Mar 16, 2023
@MichaelsJP MichaelsJP deleted the spring-update-to-2.7.9 branch March 16, 2023 12:08
ors general automation moved this from Review to Awaiting release Mar 16, 2023
@MichaelsJP MichaelsJP mentioned this pull request Mar 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlink jlink jlink approved these changes

@takb takb takb approved these changes

Assignees

@MichaelsJP MichaelsJP

Labels
None yet
Projects
ors general
  
Awaiting release
Milestone
Release 7.1.0
Development

Successfully merging this pull request may close these issues.

Spring Shell RCE inquiry updating to latest spring and spring-boot versions
3 participants
@MichaelsJP @jlink @takb