Thanks all. Hope clear all conflicts. Jon, can you now merge the PR? Regards. Gurkan
On Mon, Jan 7, 2019 at 5:21 PM Jean-Louis Monteiro <[email protected]> wrote: > Yes I agree and take it as you describe so it's all good. It's definitely a > step forward > > Le lun. 7 janv. 2019 à 12:21, Gurkan Erdogdu <[email protected]> a > écrit : > > > Hello Jean-Louis and team, > > I want to emphasize again that this PR will not change anything regarding > > system properties. Its sole aim is to centralize all literal system > > properties into its own module. Maybe, for the future, if needed, we can > > update this module to add more configuration related functions. Currently > > all literal constants are distributed around the codebase. Before adding > > any more property, we need to discuss it first in here. So, one can > easily > > find the used property this way. > > Regards. > > Gurkan > > > > On Mon, Jan 7, 2019 at 1:30 PM Jean-Louis Monteiro < > > [email protected]> > > wrote: > > > > > Just want to make sure we don't forget System Properties were meant to > be > > > used to override configuration and not to be the main configuration > > system > > > for TomEE. > > > > > > We can discuss it and decide to change our mind and TomEE, but as per > > now, > > > I'm not really keen to relying on system properties to configure TomEE. > > I'd > > > rather take the opportunity to yank some system properties when > possible. > > > > > > > > > > > > -- > > > Jean-Louis Monteiro > > > http://twitter.com/jlouismonteiro > > > http://www.tomitribe.com > > > > > > > > > On Fri, Jan 4, 2019 at 7:28 AM Gurkan Erdogdu <[email protected]> > > wrote: > > > > > > > Thanks Jon. > > > > > > > > I don't have any aim to replace service-jar.xml approach. We will > just > > > add > > > > another YAML based configuration support. Therefore, all tomee.xml, > > > > resources.xml etc will be stay in there. YAML is just an additional > > > > feature. > > > > > > > > Introducing new module , tomee-config, allow us to centralise all > > > > configuration into one place and all other modules can use it via > > > > dependency. In the future, we can remove all configuration codes from > > > > openejb-core, etc. into this module. There are lots of system > > properties > > > > using in different modules, therefore each such module will add their > > > > properties into TomEESystemConfig class and then use it. > > > > > > > > Currently, openejb-core is a one large module. We may also think to > > > divide > > > > this module into smaller modules which are having special purpose. > > > > > > > > Hope it clears your concerns. > > > > > > > > Regards. > > > > Gurkan > > > > > > > > On Thu, Jan 3, 2019 at 11:30 PM Jonathan Gallimore < > > > > [email protected]> wrote: > > > > > > > > > I commented on your PR - thanks for sending that over. I think it > > would > > > > be > > > > > worthwhile structuring the class with the constants in such a way > > that > > > > the > > > > > javadoc could end up on the website via David's site generation > code. > > > > That > > > > > would be extremely cool and I'm sure a very useful piece of > > > > documentation. > > > > > > > > > > While I appreciate your service-jar.xml visibility comment, I > suspect > > > > that > > > > > is because of something missing in its documentation, as opposed to > > an > > > > > issue with the design choice. I don't necessarily object to > something > > > > that > > > > > can parse the config in YAML (I believe there is something that > > handles > > > > > JSON already), providing the existing config mechanism in XML, with > > the > > > > > defaults in service-jar.xml continued to work, and that I wouldn't > be > > > > > forced to migrate from XML to YAML. Many users make use of > tomee.xml > > > and > > > > > resources.xml in a huge number of applications. Forcing a migration > > on > > > > them > > > > > to YAML "just because" doesn't make sense to me. The concept of the > > > > > service-jar.xml is a very core design concept in TomEE and I would > > not > > > be > > > > > favour of changing it unless there was a really good rationale and > a > > > > better > > > > > design proposed and discussed. > > > > > > > > > > I agree that system properties can lead to "magic" parameters that > > > aren't > > > > > well documented, and I'm definitely in favour of improving the > > > > > documentation of those properties. The ability to override config > > with > > > > > system properties, however, is useful and also widely used so we'll > > > need > > > > to > > > > > keep it. > > > > > > > > > > Jon > > > > > > > > > > On Wed, Jan 2, 2019 at 9:53 AM Gurkan Erdogdu <[email protected] > > > > > > wrote: > > > > > > > > > > > For me, using services-jar.xml approach is not so visible to > users. > > > All > > > > > > defaults are configured in this file and packaged within JAR > file. > > > > Users > > > > > > are not able to read the parameter explanation from > > services-jar.xml > > > > > files. > > > > > > Also, services.-jar.xml is somebit different from using the > system > > > > > > properties to turn-on/off something. I am also thinking to > > introduce > > > > YAML > > > > > > based configuration. > > > > > > > > > > > > But first step is to centralise all of these system parameters > into > > > two > > > > > > classes. Maybe, we will see that some of them are unnecessary > etc. > > > > > > > > > > > > On Wed, Jan 2, 2019 at 12:44 PM Bruno Baptista < > [email protected] > > > > > > > > wrote: > > > > > > > > > > > > > Yes, there is. > > > > > > > > > > > > > > This is also the most basic MP spec and nothing prevents us > from > > > > using > > > > > > > it everywhere. > > > > > > > > > > > > > > There might be Jakarta EE restrictions in how to handle > > > > configurations > > > > > > > that need to be assessed. > > > > > > > > > > > > > > Overall, I think that if we are going to mess with configs, we > > > should > > > > > > > use state of the art. > > > > > > > > > > > > > > Cheers > > > > > > > > > > > > > > Bruno Baptista > > > > > > > https://twitter.com/brunobat_ > > > > > > > > > > > > > > > > > > > > > On 02/01/19 09:35, Jean-Louis Monteiro wrote: > > > > > > > > I think with microprofile-config we may have a chicken and > the > > > egg > > > > > > > problem, > > > > > > > > isn't it? > > > > > > > > -- > > > > > > > > Jean-Louis Monteiro > > > > > > > > http://twitter.com/jlouismonteiro > > > > > > > > http://www.tomitribe.com > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Jan 2, 2019 at 10:30 AM Bruno Baptista < > > > [email protected] > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > >> Hi Gurkan, > > > > > > > >> > > > > > > > >> I agree we have a problem with the documentation of the > > > different > > > > > > > >> properties and that we need to improve it. > > > > > > > >> > > > > > > > >> Doing the inventory and using the proposed syntax looks ok > to > > me > > > > > but I > > > > > > > >> also think we should go even further. > > > > > > > >> > > > > > > > >> How about to migrate all the properties to use > > > > microprofile-config? > > > > > > > >> > > > > > > > >> Cheers. > > > > > > > >> > > > > > > > >> Bruno Baptista > > > > > > > >> https://twitter.com/brunobat_ > > > > > > > >> > > > > > > > >> > > > > > > > >> On 02/01/19 07:20, Gurkan Erdogdu wrote: > > > > > > > >>> Hello > > > > > > > >>> There are lots of known and unknown system properties in > the > > > > > current > > > > > > > code > > > > > > > >>> base. I would like to introduce TomEESystemProperties and > > > > > > > >>> OpenEJBSystemProperties classes to hold these system > property > > > > > > constants > > > > > > > >> and > > > > > > > >>> provide clear comment what it does. For example: > > > > > > > >>> > > > > > > > >>> class TomEESystemProperties{ > > > > > > > >>> public static final String TOMEE_FORCE_RELOADABLE = > > > > > > > >>> "tomee.force-reloadable"; > > > > > > > >>> .... > > > > > > > >>> } > > > > > > > >>> > > > > > > > >>> class OpenEJBSystemProperties{ > > > > > > > >>> public static final String > > OPENEJB_CROSSCONTEXT_PROPERTY = > > > > > > > >>> "openejb.crosscontext"; > > > > > > > >>> .... > > > > > > > >>> } > > > > > > > >>> > > > > > > > >>> WDYT? > > > > > > > >>> Regards. > > > > > > > >>> Gurkan > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > >
