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

Confusing when user provides app config but plugin generates one anyway because of location mismatch #1780

Open
scottkurz opened this issue Dec 13, 2023 · 2 comments

Comments

@scottkurz
Copy link
Member

scottkurz commented Dec 13, 2023

BACKGROUND

The context behind the issue is outlined here:
https://stackoverflow.com/questions/77548432/open-liberty-context-root-not-found-error-classcastexception-cwwkz0013e/77548433#77548433
which I wrote up as a Q&A after seeing a couple people internally stumble over related aspects of this issue.

As mentioned there, an easy way to trip over this is to configure a location like:

 <application location="demo.war"...

without configuring the non-default final name in pom.xml <finalName>${project.artifactId}</finalName> (the default final name includes the version).

Though there are clues to what's gone wrong:

[WARNING] CWWKM2179W: The application is not defined in the server configuration but the POM configuration indicates it should be installed in the apps folder. Application configuration is being added to the target server configuration dropins folder by the plug-in.
...
[INFO] [ERROR   ] CWWKZ0013E: It is not possible to start two applications called intro.

it might take too much of a trained eye to make sense of these.

PROPOSAL

Suggestion, maybe we should "fail fast" and abort the execution with an error.

IIUC, the match is currently done based on the 'location' attr of the app element. Perhaps we could do a check like: if either the 'name' or the 'id' LMP would generate for the app deployment matches something already in use, then just fail the deploy.

This might be better than allowing the app to start but without the desired config...leading to the wrong ctx root, ClassCastExc, as mentioned.

Probably could use another quick round of design review before finalizing whatever implementation we come up with.

Note from the issue history there have been a few now overlapping this area:
https://github.com/OpenLiberty/ci.maven/issues?q=is%3Aissue+CWWKZ0013E
(not that that experience argues 100% for doing what I'm proposing here..but at least argues it's an area worthy of further consideration).

Perhaps we'd want a similar ci.gradle issue?

@cherylking
Copy link
Member

Just noting here that we have to be careful with this change as someone could be depending on the current behavior. We have to be confident that the change to fail fast only occurs for a situation where the app will not function correctly because of mismatched config.

@scottkurz
Copy link
Member Author

Just noting here that we have to be careful with this change as someone could be depending on the current behavior. We have to be confident that the change to fail fast only occurs for a situation where the app will not function correctly because of mismatched config.

And/or, we could create a new config parm to disable the new behavior. Guess we'd have to see what the tough cases would look like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants