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

WIP: Migrate to Jakarta EE 8 #4397

Closed
wants to merge 2 commits into from
Closed

WIP: Migrate to Jakarta EE 8 #4397

wants to merge 2 commits into from

Conversation

erikgb
Copy link
Contributor

@erikgb erikgb commented Feb 3, 2023 •
edited
Loading

This is a draft PR suggesting migration to Jakarta EE 8, which is the first Jakarta EE release and the only one compatible with Java EE. I think this is a good first step towards migrating to Jakarta EE 9, which adds the additional complexity of the javax->jakarta API namespace move - which is what I am actually looking for. 😄

Relates to #3559

CC: @abrokenjester @hmottestad

Could this really be this simple, or are there some hidden complexities that I cannot see?

GitHub issue resolved: #

Briefly describe the changes proposed in this PR:

PR Author Checklist (see the contributor guidelines for more details):

  • my pull request is self-contained
  • I've added tests for the changes I made
  • I've applied code formatting (you can use mvn process-resources to format from the command line)
  • I've squashed my commits where necessary
  • every commit message starts with the issue number (GH-xxxx) followed by a meaningful description of the change

erikgb
erikgb commented Feb 3, 2023
View reviewed changes
tools/pom.xml Outdated Show resolved Hide resolved
@hmottestad
Copy link
Contributor

This should probably be targeted at develop.

@erikgb
Copy link
Contributor Author

erikgb commented Feb 13, 2023

This should probably be targeted at develop.

Probably yes, but I suspect we cannot upgrade to Jakarta EE 8 because of no WireMock version supporting that. Seems like they went straight for Jakarta EE 9.

@abrokenjester
Copy link
Contributor

abrokenjester commented Feb 14, 2023 via email

@barthanssens
Copy link
Contributor

If wiremock is used in the spring-part, e.g https://rest-assured.io with springmockvc could be an alternative

@erikgb
Copy link
Contributor Author

erikgb commented Feb 15, 2023

Fwiw we use WireMock in a very limited part of the project only and despite me introducing it, I've never really been a massive fan of it. If we can rework those tests to use something else I don't mind just ripping WireMock out (not sure if I'm opening a can of worms here though)

I'll support ripping WireMock out, mainly because it is tightly coupled with Java/Jakarta EE and Jetty. Maybe a spring-managed dependency could be used as a replacement - even in core modules - as this affects the test classpath only? Maybe create an issue for it?

@erikgb
Copy link
Contributor Author

erikgb commented Feb 15, 2023

Maybe https://www.mock-server.com/ could be another alternative to (closer to) WireMock. I don't have any experience with https://rest-assured.io/, but it seems to solve a bit different use case than WireMock/MockServer.

@erikgb
Copy link
Contributor Author

erikgb commented Feb 15, 2023

I created a POC draft PR for migrating to MockServer: https://github.com/eclipse/rdf4j/pull/4429/files

It seems like we will get more verbose code, but the migration is fairly simple. I could finalize this if we decide to head in that direction. @abrokenjester @barthanssens @hmottestad

WireMock seems to be a blocker for a couple of tasks at present.....

@erikgb erikgb mentioned this pull request Feb 16, 2023
Closed
@erikgb erikgb force-pushed the jakarta-ee-8 branch 2 times, most recently from 3b9f0da to dbc818b Compare February 16, 2023 13:36
@erikgb erikgb changed the base branch from main to develop February 16, 2023 13:36
@erikgb erikgb changed the title Migrate to Jakarta EE 8 WIP: Migrate to Jakarta EE 8 Feb 16, 2023
@erikgb erikgb force-pushed the jakarta-ee-8 branch 4 times, most recently from b504a6e to ff7e558 Compare February 19, 2023 17:29
@erikgb
Copy link
Contributor Author

erikgb commented Feb 19, 2023

Further progress is currently blocked by #4439. Would be good to have that PR reviewed and merged.

@erikgb
Copy link
Contributor Author

erikgb commented Feb 19, 2023 •
edited
Loading

Further progress is currently blocked by #4439. Would be good to have that PR reviewed and merged.

Probably also semi-blocked by #4437. At least it would be a lot simpler to upgrade Jetty (or migrate to Tomcat) if using JUnit Server. Actually, I think migrating to Tomcat for our test-servers would make a lot of sense because Elastic/Solr seem to depend on Jetty - which makes it harder to upgrade. Ref. the failing integration-tests: https://github.com/eclipse/rdf4j/actions/runs/4217370690/jobs/7321097966

@erikgb erikgb force-pushed the jakarta-ee-8 branch 6 times, most recently from 7023906 to b999878 Compare February 21, 2023 11:54
@erikgb erikgb mentioned this pull request Feb 21, 2023
Open
@erikgb erikgb force-pushed the jakarta-ee-8 branch 2 times, most recently from a9a4daf to 2778958 Compare March 5, 2023 10:57
@erikgb erikgb force-pushed the jakarta-ee-8 branch 3 times, most recently from 44da546 to 27991f0 Compare March 5, 2023 12:26
@erikgb erikgb force-pushed the jakarta-ee-8 branch 4 times, most recently from 9c3d835 to 43f3d8b Compare May 17, 2023 18:05
@erikgb erikgb force-pushed the jakarta-ee-8 branch 2 times, most recently from 3c75595 to 115c43d Compare May 18, 2023 18:47
@erikgb erikgb marked this pull request as ready for review May 18, 2023 19:37
@erikgb
Copy link
Contributor Author

erikgb commented May 18, 2023 •
edited
Loading

Wow, CI is finally green! A minor hack for the conflicting Jetty dependency pulled in from the Solr dependencies. I really really would like some input on how to get this change in somehow. So before I create a new PR and probably an issue, it would be very nice to know what you think:

  • Which branch should I target? This is probably a breaking change for some users - as JavaEE/JakartaEE is upgraded by one major version.
  • Is the suggested hack to overcome the Solr -> Jetty dependencies ok? It is really painful to share direct dependencies with some of our dependencies. Can this be avoided somehow? Maybe @barthanssens can take a look at the major upgrade of Solr required to make the JavaEE/JakartaEE upgrade smoother.

I've found the following issues relevant to this PR: #3559 #4252 #4441

@abrokenjester @hmottestad

@erikgb erikgb mentioned this pull request May 22, 2023
Open
@JervenBolleman
Copy link
Contributor

@erikgb as the branch develop is now targeting RDF4j 5.x I think that is the right branch to target.
I would also not mind if you go straight to jakarta ee 9 with this change now.

@erikgb
Copy link
Contributor Author

erikgb commented May 23, 2023

I would also not mind if you go straight to jakarta ee 9 with this change now.

I am not sure if that will work. Jakarta EE 9 migrates all classes from javax.* to jakarta.*, so that would be A LOT more changes. Also there is no version of Solr that support Jakarta EE 9.

@erikgb erikgb mentioned this pull request May 27, 2023
Open
@erikgb
Copy link
Contributor Author

erikgb commented May 27, 2023

Superseded by #4604

@erikgb erikgb closed this May 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@erikgb @hmottestad @abrokenjester @barthanssens @JervenBolleman