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 > > > > > > >>> > > > > > > > > > > > > > > > > > > > > >
