You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by David Blevins <da...@gmail.com> on 2015/11/11 23:59:32 UTC
SystemInstance polluting System.getProperties
Looks like we made as part of a routine Tomcat upgrade to change SystemInstance so that it modifies the System.getProperties
- commit: https://github.com/apache/tomee/commit/d9b5ea3f7218dc4bad1ebac7b8b674823fa3fa4c#diff-f51fa1817d7f3bd166acc0ea1b4f6b69 <https://github.com/apache/tomee/commit/d9b5ea3f7218dc4bad1ebac7b8b674823fa3fa4c#diff-f51fa1817d7f3bd166acc0ea1b4f6b69>
- issue: https://issues.apache.org/jira/browse/TOMEE-740 <https://issues.apache.org/jira/browse/TOMEE-740>
The goal for SystemInstance was specifically to remove dependency on System.getProperties so that embedded containers could be booted, used and discarded without polluting the System properties. This was all done as part of OpenEJB 1’s integration with Tomcat 4 (who feels old now?) which allowed you to have one embedded EJB container per webapp and also leveraged to allow cleaner Java SE integration so embedded EJB containers could be used and discarded with no scrubbing of the system properties.
Saw some offline discussion saying “we need to clean up the system properties because the SystemInstance modifies them”, so thought it’s a good time to have a chat :)
Any feedback on:
- What motivated the change?
- How do we rework that code?
Ideally, we’d restore the ability to sequentially boot embedded containers, passing in properties and not have the actual properties used reflect some aggregate of all the containers that came before.
--
David Blevins
http://twitter.com/dblevins
http://www.tomitribe.com
Re: SystemInstance polluting System.getProperties
Posted by Romain Manni-Bucau <rm...@gmail.com>.
that is fine to do it outside while it work for:
- ApplicationComposer
- TomEE
- TomEE Embedded
- Arquillian
- EJBContainer
- old fashion InitialContext
at least
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber
<http://www.tomitribe.com>
2015-11-12 15:14 GMT-08:00 David Blevins <da...@gmail.com>:
> Not sure I follow the answer. The intent of SystemInstance was to create
> a bucket separate from System.getProperties() — explicitly to not merge
> them together.
>
> If there was some desire to modify actual System properties, it seems we
> can easily do this outside the SystemInstance.
>
>
> -David
>
> > On Nov 11, 2015, at 3:08 PM, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
> >
> > Not sure why this change has been along this particular commit but the
> need
> > was to be able to leverage the aggregation of properties tomee does on
> > properties designed as "system properties" in system properties.
> >
> > I have no issue saving initial system properties and resetting them with
> > the SystemInstance but I would be concerned not merging system properties
> > with system properties (intentionally phrased this way). This is widely
> > used AFAIK cause most of libraries rely on System.getProperty and
> > conf/system.properties - to not speak about other locations - is quite
> > heavily used by users in such a manner in tests but in prod as well.
> >
> > As a summary: -1 to not merge anymore, +1 to save initial properties and
> > restore them after (with the traditional flag to not do it if a user was
> > relying on it in a test suite - wouldnt happen in prod I think).
> >
> >
> > Romain
> >
> > 2015-11-11 14:59 GMT-08:00 David Blevins <da...@gmail.com>:
> >
> >> Looks like we made as part of a routine Tomcat upgrade to change
> >> SystemInstance so that it modifies the System.getProperties
> >>
> >> - commit:
> >>
> https://github.com/apache/tomee/commit/d9b5ea3f7218dc4bad1ebac7b8b674823fa3fa4c#diff-f51fa1817d7f3bd166acc0ea1b4f6b69
> >> <
> >>
> https://github.com/apache/tomee/commit/d9b5ea3f7218dc4bad1ebac7b8b674823fa3fa4c#diff-f51fa1817d7f3bd166acc0ea1b4f6b69
> >>>
> >> - issue: https://issues.apache.org/jira/browse/TOMEE-740 <
> >> https://issues.apache.org/jira/browse/TOMEE-740>
> >>
> >> The goal for SystemInstance was specifically to remove dependency on
> >> System.getProperties so that embedded containers could be booted, used
> and
> >> discarded without polluting the System properties. This was all done as
> >> part of OpenEJB 1’s integration with Tomcat 4 (who feels old now?) which
> >> allowed you to have one embedded EJB container per webapp and also
> >> leveraged to allow cleaner Java SE integration so embedded EJB
> containers
> >> could be used and discarded with no scrubbing of the system properties.
> >>
> >> Saw some offline discussion saying “we need to clean up the system
> >> properties because the SystemInstance modifies them”, so thought it’s a
> >> good time to have a chat :)
> >>
> >> Any feedback on:
> >>
> >> - What motivated the change?
> >> - How do we rework that code?
> >>
> >> Ideally, we’d restore the ability to sequentially boot embedded
> >> containers, passing in properties and not have the actual properties
> used
> >> reflect some aggregate of all the containers that came before.
> >>
> >>
> >> --
> >> David Blevins
> >> http://twitter.com/dblevins
> >> http://www.tomitribe.com
> >>
> >>
>
>
Re: SystemInstance polluting System.getProperties
Posted by David Blevins <da...@gmail.com>.
Not sure I follow the answer. The intent of SystemInstance was to create a bucket separate from System.getProperties() — explicitly to not merge them together.
If there was some desire to modify actual System properties, it seems we can easily do this outside the SystemInstance.
-David
> On Nov 11, 2015, at 3:08 PM, Romain Manni-Bucau <rm...@gmail.com> wrote:
>
> Not sure why this change has been along this particular commit but the need
> was to be able to leverage the aggregation of properties tomee does on
> properties designed as "system properties" in system properties.
>
> I have no issue saving initial system properties and resetting them with
> the SystemInstance but I would be concerned not merging system properties
> with system properties (intentionally phrased this way). This is widely
> used AFAIK cause most of libraries rely on System.getProperty and
> conf/system.properties - to not speak about other locations - is quite
> heavily used by users in such a manner in tests but in prod as well.
>
> As a summary: -1 to not merge anymore, +1 to save initial properties and
> restore them after (with the traditional flag to not do it if a user was
> relying on it in a test suite - wouldnt happen in prod I think).
>
>
> Romain
>
> 2015-11-11 14:59 GMT-08:00 David Blevins <da...@gmail.com>:
>
>> Looks like we made as part of a routine Tomcat upgrade to change
>> SystemInstance so that it modifies the System.getProperties
>>
>> - commit:
>> https://github.com/apache/tomee/commit/d9b5ea3f7218dc4bad1ebac7b8b674823fa3fa4c#diff-f51fa1817d7f3bd166acc0ea1b4f6b69
>> <
>> https://github.com/apache/tomee/commit/d9b5ea3f7218dc4bad1ebac7b8b674823fa3fa4c#diff-f51fa1817d7f3bd166acc0ea1b4f6b69
>>>
>> - issue: https://issues.apache.org/jira/browse/TOMEE-740 <
>> https://issues.apache.org/jira/browse/TOMEE-740>
>>
>> The goal for SystemInstance was specifically to remove dependency on
>> System.getProperties so that embedded containers could be booted, used and
>> discarded without polluting the System properties. This was all done as
>> part of OpenEJB 1’s integration with Tomcat 4 (who feels old now?) which
>> allowed you to have one embedded EJB container per webapp and also
>> leveraged to allow cleaner Java SE integration so embedded EJB containers
>> could be used and discarded with no scrubbing of the system properties.
>>
>> Saw some offline discussion saying “we need to clean up the system
>> properties because the SystemInstance modifies them”, so thought it’s a
>> good time to have a chat :)
>>
>> Any feedback on:
>>
>> - What motivated the change?
>> - How do we rework that code?
>>
>> Ideally, we’d restore the ability to sequentially boot embedded
>> containers, passing in properties and not have the actual properties used
>> reflect some aggregate of all the containers that came before.
>>
>>
>> --
>> David Blevins
>> http://twitter.com/dblevins
>> http://www.tomitribe.com
>>
>>
Re: SystemInstance polluting System.getProperties
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Not sure why this change has been along this particular commit but the need
was to be able to leverage the aggregation of properties tomee does on
properties designed as "system properties" in system properties.
I have no issue saving initial system properties and resetting them with
the SystemInstance but I would be concerned not merging system properties
with system properties (intentionally phrased this way). This is widely
used AFAIK cause most of libraries rely on System.getProperty and
conf/system.properties - to not speak about other locations - is quite
heavily used by users in such a manner in tests but in prod as well.
As a summary: -1 to not merge anymore, +1 to save initial properties and
restore them after (with the traditional flag to not do it if a user was
relying on it in a test suite - wouldnt happen in prod I think).
Romain
2015-11-11 14:59 GMT-08:00 David Blevins <da...@gmail.com>:
> Looks like we made as part of a routine Tomcat upgrade to change
> SystemInstance so that it modifies the System.getProperties
>
> - commit:
> https://github.com/apache/tomee/commit/d9b5ea3f7218dc4bad1ebac7b8b674823fa3fa4c#diff-f51fa1817d7f3bd166acc0ea1b4f6b69
> <
> https://github.com/apache/tomee/commit/d9b5ea3f7218dc4bad1ebac7b8b674823fa3fa4c#diff-f51fa1817d7f3bd166acc0ea1b4f6b69
> >
> - issue: https://issues.apache.org/jira/browse/TOMEE-740 <
> https://issues.apache.org/jira/browse/TOMEE-740>
>
> The goal for SystemInstance was specifically to remove dependency on
> System.getProperties so that embedded containers could be booted, used and
> discarded without polluting the System properties. This was all done as
> part of OpenEJB 1’s integration with Tomcat 4 (who feels old now?) which
> allowed you to have one embedded EJB container per webapp and also
> leveraged to allow cleaner Java SE integration so embedded EJB containers
> could be used and discarded with no scrubbing of the system properties.
>
> Saw some offline discussion saying “we need to clean up the system
> properties because the SystemInstance modifies them”, so thought it’s a
> good time to have a chat :)
>
> Any feedback on:
>
> - What motivated the change?
> - How do we rework that code?
>
> Ideally, we’d restore the ability to sequentially boot embedded
> containers, passing in properties and not have the actual properties used
> reflect some aggregate of all the containers that came before.
>
>
> --
> David Blevins
> http://twitter.com/dblevins
> http://www.tomitribe.com
>
>