-
Notifications
You must be signed in to change notification settings - Fork 167
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
Bump project to use spring boot 2.x (Spring 1.x is EOL) #2056
Comments
Hello my name is Edwin M. Cruz, a newbie staff member of Columbia University DBMI team. I'm an experienced Spring/Java engineer. I would love to become a contributor of this project. My work email is ec669@cumc.columbia.edu. |
This is definitely something we want to address, here are some other open issues related to dependencies: I could have sworn that there were other issues that were related to updating library dependencies (I thought shrio was one) but I only searched through open issues). I could have sworn @t-abdul-basser did a summary of issues that he encountered when trying to update the Spring dependency, but I can't seem to find it. We can use this issue to specifically track updating Spring boot to 2.x. But there are a number of dependency updates that we'd like to tackle in 1 release of Atlas (and considering the dependency changes, it will probably be the start of a 3.x line of Atlas):
I'd prefer to handle groups of dependencies in targeted PRs, but I expect that updating one dependency (such as Spring to 2.x) will involve a newer JDK, which will require other libraries to migrate in a big cascading dependency migration in 1 go. Fortunately, we do have a good set of test cases that we can ensure the app behavior is maintained, but we probably don't have enough test coverage to be completely confident that everything 'just works' after making the changes. |
Spring Boot 2.7 supports java 8 if i recall correctly, but Spring Boot 3 will no longer support java 8 - so i would say stay on java 8 for now. I'll dig deeper and create/update list of libs and their ages. Once we are fully on Spring Boot 2.x and tested we can push forward for java 11 then 17. I'm a frontend ninja as well and have some ideas for frontend as well. |
Thanks for welcoming @chrisknoll! @ecruz165 is a new-ish member of our team that will be working with us on OHDSI Atlas and WebAPI. Would you add him as a participant to the OHDSI Github org? Thanks. |
Done. |
@chrisknoll , @t-abdul-basser , 1. Bump several libraries that line up with dependency versions provided by Spring Boot 2.7.Status: Partially Merged - Pull Request
Proposed PR Sets
6. Bump usage of JUnit 4 to JUnit 5 (Story Points: 8)
7. Bump Hibernate Libs to 5.6.10.Final (Story Points: 3)
8. Bump Spring Boot to version 2.x and <=2.3 (Story Points: 3)9. Bump remaining aged libraries that are not managed by Spring Boot (Story Points: 2)10. Upgrade to Spring Boot 2.4 (Story Points: 3)11. Upgrade to Spring Boot 2.7 (Story Points: 3)12. Add Code Coverage report generation (Story Points: 1)13. Bump JDK to Java 11 (Story Points: 5)14. Bump JDK to Java 17 (Story Points: 3)15. Analyze code with Sonar Lint and resolve issues (Story Points: 5)16. Introduce Open Docs API, Swagger, and Actuator Endpoints (Story Points: 3)17. Monorepo Project Layout (Story Points: 5)18. Mocked Integrations tests (Story Points: 5)19. Upgrade to Spring Boot 3.0. (Being released End of December) (Story Points: 8)20. Upgrade JDK to Java 19 (Story Points: 3) |
Edwin, thank you for the roadmap compilation and sharing your thoughts, they look very detailed @ecruz165 You mentioned you will have some dedicated time for WebAPI / ATLAS development in the upcoming months. |
Alex, thank you for reviewing the road map - almost thought it might not be discussed/viewed. And apologies for my overly ambitious PR on the first contribution. I am new to contributing to an open-source project. I would like to say I can safely promise 3 full working days at a minimum. I believe mid/late December would be possible. If things go smoothly, maybe faster. Every improvement would be a PR, as described above. That would be about 16 PRs. I would like to know what your checkout process is so I can do the same before creating a PR. I have all but 2 unit tests passing, and the 2 flaky unit tests do pass when run alone. Currently, the IT tests don't run on my setup. Maybe I am missing data cause the results are empty. And another thing, I had to look for a workaround to run tests with embedded Postgres, maybe cause I work on a mac. @alex-odysseus @chrisknoll @t-abdul-basser. |
So I might have been unclear about separate PR's in other threads, so I don't want to confuse anyone about when we want separate PRs: We can bundle these PRs together as one if the topic of a PR is to update dependencies. I believe that in other PRs where I've asked for separate PRs, it was because there was a suggestion to make an additional change unrelated to the topic, and I asked that it be dealt with separately. One reason for separate PRs in this context is that if we have lower risk dependencies that can be grouped together, while an other series of PRs that are higher risk can be created, such that we can get a bunch of dependencies fixed on one shot, but then we have a set of other prs where half may work out, and the other half may not...if they were all in 1 PR we'd hold up 50% of fixes beccause of challenges in the other 50%. So, if you think we can nail all of them in 1 PR, fine. If we can group these into a few PRs because it makes logical sense to group them, also fine. I don't think we need to have 1 pr per dependency, that would be cumbersome and probably unnecessary. |
I updated work sets to mitigate risk with estimated story points and did a minor reorg based on what I believe is most important. @chrisknoll @alex-odysseus @t-abdul-basser |
Thanks @ecruz165! |
Expected behavior
Platform should run all functions successfully using Spring 2.x
Actual behavior
Upgrading libraries breaks build.
Steps to reproduce behavior
Change spring boot version to 2.x and project will not compile.
The text was updated successfully, but these errors were encountered: