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
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.
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.
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?
WDYT
On Tue, Jul 18, 2017 at 11:11 PM, gil portenseigne
<[email protected]> wrote:
> Hello Guys,
>
> I just created https://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
>>
>
