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 <david.blev...@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 <rmannibu...@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 <david.blev...@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 > >> > >> > >
