Hello Taher,
My thoughts inline :)
On 20/07/2017 09:05, Taher Alkhateeb wrote:
Hello Gil,
Thank you for working on this. I have reviewed the patch and I have a few notes:
First for gradle, I don't think you need to fetch environment
variables. You can do whatever you want by passing flags, which might
be a simpler solution. So you create your own script and modify gradle
for your environment. You can modify the project with -P flags and the
JVM with -D flags
We did the dev into a 13.07, so without gradle. I just re-did in gradle
what we have done with ant with our needs.
I agree we should use -D flags instead of env variable :).
Now with respect to the rest of the work, I'm not sure but I think
this makes the code a little more fragile or brittle. Essentially you
are injecting multiple code locations with variables that you fetch
from the environment.
Yes there are multi location modifications since we wanted to configure
not only classic property files, but entityengine.xml,
serviceengine.xml, web.xml, etc.
I do not measure the risk of such modifications, that you're talking
about, since default value are available.
But we indeed feel the need to check what configuration is available
within OFBiz to ensure its validity (hosting company job)... that is
another kind of fragility.
This means you are putting "state" inside application logic. One of
thepain points for me when refactoring OFBiz was that I change some
code somewhere and I get something going in flames in a completely
different location. Why does that happen? Because of this sort of
_secret_ shared state.
I understand your feeling and opinion, but i am not sure that these
small modifications are going to create flames in the app :).
I cannot yet visualize the "state" you mentioned, now it's just a
improvement of the way to get properties value, that should be use only
in very controlled production environment.
I will re-check if there is a way to minimize the several modification
locations.
Why not consider a different approach like either template generation
or multiple files. For example, instead of fetching variables and
introducing a new notation in the source files, instead you can have
test.general.properties, prod.general.properties,
develop.general.properties and so on. Alternatively, you can add a
template using freemarker, and make a new gradle task (e.g.
generateConfigurationFiles).
So for example, you can have something like this:
./gradlew generateConfigurationFiles -Penvironment=test
./gradlew generateConfigurationFiles -Penvironment=production
./gradlew generateConfigurationFiles -Penvironment=development
./gradlew generateConfigurationFiles -PsourceTemplate=mySourceTemplate.ftl
./gradlew generateConfigurationFiles -PoverrideProperties=myOverride.properties
Something like this would give you the ability to customize without
adding more moving parts. Wouldn't that achieve the same results
you're seeking? It might also have the advantage of maintaining
existing notation instead of adding this ${env: ...} notation?
The original needs was to avoid modifying files in production (via
patches/addons), to let hosting company sys admin to manage the
configurations changes within the environment without touching code base.
With your proposal, first we have to explain to them how to apply
specific configuration using gradle (not a big deal), then my concern is
that these template will have to be maintained by the sys admin, i.e.
when we are adding some config in a property file and this one is
already templated, communication is needed to explain what template
should be adapted etc.
With our proposal, no communication with the hosting company is needed,
but only when new environment variable is needed, and, no code base
modification is done on the VM after delivery.
I like the template proposal (currently we use git branches to manage
this need), this could be complementary with the "environment notation".
Thanks for your feedback !
Gil
WDYT
On Tue, Jul 18, 2017 at 11:11 PM, gil portenseigne
<[email protected]> wrote:
Hello Guys,
I just createdhttps://issues.apache.org/jira/browse/OFBIZ-9498
You can test it out with the provided patch.
And James, I just tested two launch to instances with different environment
properties with no issue (different JVM args and database connections)
Cheers
Gil
On 06/07/2017 22:07, gil portenseigne wrote:
Hi James,
inline
On 06/07/2017 12:12, James Yong wrote:
Hi Gil,
Some afterthought questions with the proposed change:
1) Can it support multiple OFBiz instances on the same server?
Using different shell process to launch the multiple OFBiz instances, i
think that it's possible to set environment variable dedicated to each OFBiz
instance.
2) Can we store encrypted values for some of the environment variables
like database credentials?
You can store whatever value you want into environment variable, but you
must adapt OFBiz to decrypt the value.
Thanks all for your feedback, i'll work on trunk to make it available soon
(within a new Jira)
Regards
Gil
