-
Notifications
You must be signed in to change notification settings - Fork 332
Add optional eclipselink dependency to build and include as build arg in Dockerfile #114
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
Conversation
polaris-service/build.gradle.kts
Outdated
|
|
||
| if (project.properties.get("eclipseLink") == "true") { | ||
| dependencies { | ||
| implementation(project(":polaris-eclipselink")) |
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.
Would it harm to unconditionally include it as a runtimeOnly dependency?
For integration-testing, we could also add a separate test-suite to :polaris-service that has :polaris-eclipselink as well.
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.
It's relatively easy to add such a test-suite:
testing {
suites {
register("intTestEclipselink", JvmTestSuite::class.java) {
useJUnitJupiter()
dependencies {
implementation(project())
implementation(project(":polaris-eclipselink") // this would be an implementation-dependency for the suite
}
targets {
all {
tasks.named("check").configure { dependsOn(testTask) }
}
}
}
}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.
I opened #125 to discuss whether we should include the dependency by default
|
Raised similar PR for this: #118 I think we still needed to add h2 lib for default persistent backend for extension/persistence/eclipselink/build.gradle.kts. |
sfc-gh-aixu
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.
e296a3a to
da8772a
Compare
|
Merging this after discussion in #125 |
aihuaxu
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.
Description
#53 removed the eclipselink subproject as a dependency of the
polaris-servicebuild. As an optional dependency, it makes sense to not include it automatically. However, users will need a way to include the subproject in the service jar and in the output Dockerfile.This updates the gradle build to accept an optional
eclipseLinkproperty that includes the subproject in the shadow jar. Additionally, there's a correspondingECLIPSELINKarg in the docker build.While testing this, I also updated the Dockerfile to specify an
ENTRYPOINTand aCMDso that it's easy to point to a differentpolaris-server.ymlfile and/or execute a different command (such as thebootstrapor upcomingpurgecommand) from the same docker image.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Ran
docker build . --build-arg ECLIPSELINK=true -t polaris:latestthen I updated my local
polaris-server.ymlfile to specifyeclipse-linkas themetaStoreManagerand ran withdocker run -p 8181:8181 -v ${PWD}:/conf polaris:latest server /conf/polaris-server.ymlConfirmed the following was in the server output at startup:
Test Configuration:
Checklist:
Please delete options that are not relevant.