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

[WFLY-19662] Switch from / to helloworld-ws context in QS #948

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

baranowb
Copy link
Contributor

@baranowb baranowb requested a review from emmartins as a code owner August 26, 2024 11:23
@emmartins
Copy link
Contributor

Unfortunately the changes don't work with server provisioning and openshift, thus the test fails, and after some time investigating I found no way to easily support the current dual mode we have in other quickstarts, that this issue wants to implement (default deploy at /helloworld-ws for baremetal, deploy at root for openshift). I mean you either deploy always at root (current design), or you always deploy at /helloworld-ws, you can't have it working for both cases depending on wether using baremetal or not.

In theory the only changes that would be needed in this PR, besides test logic, would be to delete the jboss-web.xml, letting undertow decide whether to deploy at root or not, but if you do this then deploy at root doesn't work (this is an old issue, see https://developer.jboss.org/thread/248731). @jimma maybe you want to investigate this particular issue (why removing the jboss-web.xml no longer works for "mvn package -Pprovisioned-server" + "mvn wildfly:start") in a separate WFLY jira?

Anyway, please note that there is another JIRA that plans to do kind of related modifications, https://issues.redhat.com/browse/WFLY-19463 , more specifically the "No deployment at web root" change, which will do similar changes to this PR but also removing the ROOT.war config in the provisioned-server maven profile, and then it works for both baremetal and provisioning-server (full changes for that at https://github.com/emmartins/quickstart/pull/new/WFLY-19662 ) so I think let's just defer this fix to that JIRA on WFLY 35. WDYT @baranowb @kstekovi ?

@emmartins emmartins added the hold do not merge label Sep 24, 2024
@jimma
Copy link
Contributor

jimma commented Sep 25, 2024

@emmartins I already created https://issues.redhat.com/browse/WFLY-19780. I'll try to look more things to see if this can be fixed without breaking other things. I'll keep you updated once I find this can't be fixed without breaking other things.

@emmartins
Copy link
Contributor

thank you @jimma

@baranowb
Copy link
Contributor Author

I could have sworn Ive replied to this already. I have no preference, just something I noticed when I was working on some issue, so I filled PRs.

@kstekovi
Copy link
Contributor

Hi @emmartins

The https://issues.redhat.com/browse/WFLY-19463 is resolved. can we finish this one?

@emmartins emmartins closed this Nov 28, 2024
@emmartins emmartins reopened this Nov 28, 2024
@emmartins emmartins merged commit d34c86c into wildfly:main Nov 28, 2024
12 of 21 checks passed
@emmartins
Copy link
Contributor

thank you @baranowb

@jimma
Copy link
Contributor

jimma commented Nov 29, 2024

@emmartins wildfly/wildfly#18290 is still not merged. Without this change, it seems this doesn't work ?

@emmartins
Copy link
Contributor

emmartins commented Nov 29, 2024 via email

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

Successfully merging this pull request may close these issues.

4 participants