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

Update starter #2

Merged
merged 15 commits into from
Oct 5, 2020
Merged

Update starter #2

merged 15 commits into from
Oct 5, 2020

Conversation

scottkurz
Copy link
Member

@scottkurz scottkurz commented Oct 1, 2020

Here we implement the UFO design from: OpenLiberty/open-liberty#9343
(in turn referencing in places the openliberty.io starter design: OpenLiberty/openliberty.io#1424).

There were too many details to discuss on the design call so here's an attempt to compromise and meet the goal of providing a starter, with useful code that will largely be used as-is and not deleted, rather than a sample.

The default starter shown here includes files:

.gitignore
LICENSE
pom.xml
src/main/java/dev/odo/starter/StarterApplication.java
src/main/liberty/config/configDropins/defaults/quick-start-security.xml
src/main/liberty/config/server.xml
src/main/webapp/WEB-INF/beans.xml
src/test/java/dev/odo/starter/it/EndpointIT.java

Some highlights:

  1. It does include a functional IT using Health endpoint. This is debatable, a user might not want this. But at least the test doesn't require any constructs in the app side of the project that would need to be deleted, even if the test itself would get deleted. Using the health endpoint to do IT in dev mode has presented problems in the past: Dev mode runs ITs exercising Health Check endpoints before they are available ci.maven#766, let's see if it's still an issue in more recent Open Liberty versions.
  2. We don't include explicit HealthCheck skeleton impls, instead we rely on the defaults.
  3. We don't define any config properties, variables for ports, instead using 9080/9443. (Had originally thought the tests could run in non-Docker Maven but they don't now, and the port is intra-container only).
  4. no explicit keystore config in server.xml - rely on generated keystore password, and use non-default "mergeServerEnv" config in liberty-maven-plugin to make sure this doesn't get overwritten
  5. beans.xml - bean discovery mode = all
  6. JAXRS Application - Use @ApplicationPath("/") to match MST default for RESTClient injection, since MST isn't currently set up to discover this annotation without additional work.
  7. hard-coded app name of "starter-default.war" (matching artifact name). Would like to fix, and use a property like the MicroProfile starter, but hit an issue. Follow-up here).
  8. GAV name
    <groupId>dev.odo.starter.java-openliberty</groupId>
    <artifactId>starter-default</artifactId>
    <version>1.0-SNAPSHOT</version>
    <packaging>war</packaging>

@gcharters @yeekangc @gkwan-ibm @Emily-Jiang @NottyCode

Copy link

@ajm01 ajm01 left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Scott Kurz <[email protected]>
@scottkurz scottkurz merged commit f65097b into OpenLiberty:master Oct 5, 2020
@scottkurz
Copy link
Member Author

This seemed more useful merged to master than holding it out but feel free to open a new issue (or otherwise contact me) with comments here.

One comment from @yeekangc internally suggested removing the IT, which we'll probably do.

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