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.

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

GH-5064: added docker makefile for Jetty 9 #5078

Merged

Conversation

barthanssens
Copy link
Contributor

@barthanssens barthanssens commented Jul 18, 2024

GitHub issue resolved: #5064

Briefly describe the changes proposed in this PR:

  • add docker makefile for Jetty 9
  • updated docker makefile for Tomcat 9 (instead of 8.5)
  • updated e2e tests to run the tests for both Jetty and Tomcat
  • minor updates of documentation
  • added leading slash in overview.jsp to redirect to correct path (Jetty)

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

@hmottestad
Copy link
Contributor

The end-to-end tests probably just build the default dockerfile. Could you have it run against both the default/current and the jetty one?

@barthanssens
Copy link
Contributor Author

Ah, good thinking, forgot about that

@barthanssens barthanssens force-pushed the GH-5064-docker-jetty-server branch 2 times, most recently from db55020 to 7e70a2e Compare July 22, 2024 18:30
@barthanssens barthanssens marked this pull request as draft July 22, 2024 18:31
@barthanssens
Copy link
Contributor Author

barthanssens commented Jul 22, 2024

Workbench seems to work, but still an issue with 404 on overview.view on the server

Jetty redirects the "home page" to rdf4j-server/WEB-INF/views/home/overview.view instead of rdf4j-server/home/overview.view

e2e/run.sh Outdated
Comment on lines 35 to 42
npx playwright test

cd ..
cd docker
./shutdown.sh
done

exit $?
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if the test fail for tomcat but not jetty? Does it still error out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmz yes, that's on the TODO list (and it probably doesn't hurt to update the docker documentation as well...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@barthanssens barthanssens force-pushed the GH-5064-docker-jetty-server branch 3 times, most recently from 0bd1d7b to a7763bf Compare July 26, 2024 12:50
@barthanssens barthanssens force-pushed the GH-5064-docker-jetty-server branch from bfee867 to f7566d0 Compare August 1, 2024 16:05
@barthanssens barthanssens marked this pull request as ready for review August 1, 2024 17:58
@hmottestad hmottestad merged commit 5ba73ef into eclipse-rdf4j:develop Nov 10, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants