search

openshift-s2i / s2i-wildfly

Source-to-Image template for WildFly applications
http://wildfly.org/
Other
73 stars 145 forks source link

Reverted the parts of 850bd50 (custom example DS) which apply to wild… #129

Closed Larzenegger closed 7 years ago

Larzenegger commented 7 years ago

…fly 8.1 (#128)

openshift-bot commented 7 years ago

Can one of the admins verify this patch?

bparees commented 7 years ago

what is the motivation for this PR?

bparees commented 7 years ago

ah, I see the issue reference now.

Larzenegger commented 7 years ago

As described in #128 , it seems impossible to deploy a wildfly 8.1 build because the datasource jndi and pool name and cannot be parsed and bound.

I can add the docker logs if that helps.

bparees commented 7 years ago

i don't think this is specific to 8.1, i think the original PR introducing this was broken for all versions.

Larzenegger commented 7 years ago

When we encountered this (we're running 8.1) and I tried narrowing down the issue, I believe I was able to deploy a wildfly 10.1 build of your sample application. Running another build to verify now.

bparees commented 7 years ago

it deploys, but if you look at the standalone.xml inside the container the value is not substituted (in 10.1).

it's possible wildfly itself is doing the substitution internally and that's why it works, but i'd prefer to make it consistent. working on a pull here: https://github.com/openshift-s2i/s2i-wildfly/pull/130

Larzenegger commented 7 years ago

https://docs.jboss.org/author/display/WFLY8/Expressions gave me the impression that we should be able to use environment variables in the configuration. It just didn't seem to work in that datasource element using version 8.1.
But of course I don't mind if you pursue another approach that solves the issue and is consistent across versions.

bparees commented 7 years ago

yeah, there's some limited support for substitution in certain fields of the wildfly config (and it may have been expanded in 10.1) but if my PR works for you, i'd prefer it since it keeps the behavior consistent. Are you able to build+test the image from my PR? It looks like it was doing the right thing when i tested it, but i'd certainly appreciate outside confirmation.

bparees commented 7 years ago

(pull is here: https://github.com/openshift-s2i/s2i-wildfly/pull/130, i originally linked the wrong one above)

Larzenegger commented 7 years ago

Resolved by #130