Very nice addons :) Thanks!
regards, François fpa...@apache.org Le 29/09/2020 à 08:23, Romain Manni-Bucau a écrit : > LGTM now, thanks for the work! > > Romain Manni-Bucau > @rmannibucau <https://twitter.com/rmannibucau> | Blog > <https://rmannibucau.metawerx.net/> | Old Blog > <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > <https://www.packtpub.com/application-development/java-ee-8-high-performance> > > > Le mar. 29 sept. 2020 à 08:10, Jean-Baptiste Onofre <j...@nanthrax.net> a > écrit : > >> Done, I’m doing a full test. >> >> Regards >> JB >> >>> Le 29 sept. 2020 à 07:59, Romain Manni-Bucau <rmannibu...@gmail.com> a >> écrit : >>> Hi JB, >>> >>> for the env part it looks exactly what I would expect but for system >>> properties I wouldnt normalize the key (uppercase+replace dots by >>> underscrores), it should just be "pid + "." + key" I think. Normalization >>> is not only to look like common practise but also cause dots are not >>> accepted in env var IIRC. Since it is not the case for system props, no >>> reason to apply that transformation IMHO. >>> Last small note is that env should win over system properties so second >>> "if" should be preceeded by an "else" IMHO. >>> >>> Hope it makes sense. >>> >>> Romain Manni-Bucau >>> @rmannibucau <https://twitter.com/rmannibucau> | Blog >>> <https://rmannibucau.metawerx.net/> | Old Blog >>> <http://rmannibucau.wordpress.com> | Github < >> https://github.com/rmannibucau> | >>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book >>> < >> https://www.packtpub.com/application-development/java-ee-8-high-performance >>> >>> >>> Le mar. 29 sept. 2020 à 07:16, Jean-Baptiste Onofre <j...@nanthrax.net> a >>> écrit : >>> >>>> By the way, ConfigurationPlugin is R7 so Karaf 4.3.x. >>>> >>>> For Karaf 4.2.x, I will use the ${env} notation in cfg files using the >>>> same var names as working on 4.3. >>>> >>>> Regards >>>> JB >>>> >>>>> Le 29 sept. 2020 à 05:56, Jean-Baptiste Onofre <j...@nanthrax.net> a >>>> écrit : >>>>> Hi, >>>>> >>>>> I’ve updated the PR to use ConfigurationPlugin. >>>>> >>>>> NB: when using ConfigurationPlugin, if you use >>>> configuration.getProperties() you will have the default values (not >>>> interpolated). To get the actual property values, you have to use >>>> configuration.getProcessedProperties(). With >>>> configuration.getProcessedProperties(), you will the interpolated value. >>>>> In order to avoid to confuse users, I also updated configRepository and >>>> config:* commands to use getProcessedProperties() by default. >>>>> I’m doing a pass on other Karaf modules. >>>>> >>>>> Regards >>>>> JB >>>>> >>>>>> Le 28 sept. 2020 à 11:39, Romain Manni-Bucau <rmannibu...@gmail.com> >> a >>>> écrit : >>>>>> +1, I'd try to push for a built-in support - >>>>>> (<pid>.<property_name>).toUpperCase(ROOT).replace('.', '_') - to make >> it >>>>>> generic and not specific to some entries. It would also enable to not >>>> have >>>>>> to rename these env variables or have to maintain some fallbacks. >>>>>> What about just implementing a felix config admin configuration >> plugin? >>>>>> sounds less invasive and better on the long run? Did you give it a >> try? >>>>>> Romain Manni-Bucau >>>>>> @rmannibucau <https://twitter.com/rmannibucau < >>>> https://twitter.com/rmannibucau>> | Blog >>>>>> <https://rmannibucau.metawerx.net/ <https://rmannibucau.metawerx.net/ >>>> | Old Blog >>>>>> <http://rmannibucau.wordpress.com <http://rmannibucau.wordpress.com/ >>>> | Github <https://github.com/rmannibucau < >> https://github.com/rmannibucau>> >>>> | >>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau < >>>> https://www.linkedin.com/in/rmannibucau>> | Book >>>>>> < >> https://www.packtpub.com/application-development/java-ee-8-high-performance >>>> < >>>> >> https://www.packtpub.com/application-development/java-ee-8-high-performance >>>>>> >>>>>> >>>>>> Le lun. 28 sept. 2020 à 11:12, Jean-Baptiste Onofre <j...@nanthrax.net >>>> <mailto:j...@nanthrax.net>> a >>>>>> écrit : >>>>>> >>>>>>> Fair enough (and I agree ;)). >>>>>>> >>>>>>> Regards >>>>>>> JB >>>>>>> >>>>>>>> Le 28 sept. 2020 à 11:09, Grzegorz Grzybek <gr.grzy...@gmail.com> a >>>>>>> écrit : >>>>>>>> I'd stick to explicit approach for now and leave "implicit" one for >>>>>>> later ;) >>>>>>>> regards >>>>>>>> Grzegorz Grzybek >>>>>>>> >>>>>>>> pon., 28 wrz 2020 o 11:08 Jean-Baptiste Onofre <j...@nanthrax.net >>>> <mailto:j...@nanthrax.net> <mailto: >>>>>>> j...@nanthrax.net <mailto:j...@nanthrax.net>>> napisał(a): >>>>>>>>> Just to be clear: today I’m using an "explicit" approach, where the >>>>>>> config >>>>>>>>> contains something like: >>>>>>>>> >>>>>>>>> sshPort=${env:KARAF_SSH_PORT:-8101} >>>>>>>>> >>>>>>>>> But we can also have "implicit" approach (with some more changes >>>>>>> required). >>>>>>>>> Regards >>>>>>>>> JB >>>>>>>>> >>>>>>>>>> Le 28 sept. 2020 à 11:07, Jean-Baptiste Onofre <j...@nanthrax.net >>>> <mailto:j...@nanthrax.net>> a >>>>>>>>> écrit : >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> At beginning, I did a very simple change in Karaf Main: for >>>> instance, >>>>>>>>> you would be able to do >>>>>>>>>> Bin/karaf -Dpid:prop=value >>>>>>>>>> >>>>>>>>>> And I init the configuration file with the value. >>>>>>>>>> >>>>>>>>>> However, system properties are not super easy with docker. >>>>>>>>>> >>>>>>>>>> That’s why I preferred the env variable approach. >>>>>>>>>> >>>>>>>>>> Now, about env variable, I just leverage what we already have in >>>> Karaf >>>>>>>>> (just updating the default configuration file). >>>>>>>>>> I can do a new iteration where (in configadmin repository), I’m >>>>>>> checking >>>>>>>>> ALL env variables to find one matching. >>>>>>>>>> It would mean something like: >>>>>>>>>> >>>>>>>>>> $ export KARAF.MY_PID.prop=value >>>>>>>>>> >>>>>>>>>> For instance. >>>>>>>>>> >>>>>>>>>> We would need a "env variable format". >>>>>>>>>> >>>>>>>>>> Thoughts ? >>>>>>>>>> >>>>>>>>>> Regards >>>>>>>>>> JB >>>>>>>>>> >>>>>>>>>>> Le 28 sept. 2020 à 10:59, Grzegorz Grzybek <gr.grzy...@gmail.com >>>> <mailto:gr.grzy...@gmail.com> >>>>>>>>> <mailto:gr.grzy...@gmail.com <mailto:gr.grzy...@gmail.com> >> <mailto: >>>> gr.grzy...@gmail.com <mailto:gr.grzy...@gmail.com>>>> a écrit : >>>>>>>>>>> Hello >>>>>>>>>>> >>>>>>>>>>> Good idea! >>>>>>>>>>> >>>>>>>>>>> Shouldn't configadmin do it by default? >>>>>>>>>>> >>>>>>>>>>> Like dissect "KARAF_SSH_PORT" or similar env variables into: >>>>>>>>>>> - prefix (KARAF_) - rejected >>>>>>>>>>> - PID pointer (e.g., SSH → org.apache.karaf.shell) >>>>>>>>>>> - property (e.g PORT → sshPort) >>>>>>>>>>> ? >>>>>>>>>>> >>>>>>>>>>> This way it could be done in one place... Just my random >>>> observation, >>>>>>>>>>> because I can't dig this problem further for now ;) >>>>>>>>>>> >>>>>>>>>>> regards >>>>>>>>>>> Grzegorz Grzybek >>>>>>>>>>> >>>>>>>>>>> pon., 28 wrz 2020 o 10:51 Jean-Baptiste Onofre <j...@nanthrax.net >>>> <mailto:j...@nanthrax.net> >>>>>>> <mailto:j...@nanthrax.net <mailto:j...@nanthrax.net>> >>>>>>>>> <mailto:j...@nanthrax.net <mailto:j...@nanthrax.net> <mailto: >>>> j...@nanthrax.net <mailto:j...@nanthrax.net>>> <mailto: >>>>>>> j...@nanthrax.net <mailto:j...@nanthrax.net> <mailto:j...@nanthrax.net >>>> <mailto:j...@nanthrax.net>> <mailto:j...@nanthrax.net <mailto: >> j...@nanthrax.net> >>>> <mailto: >>>>>>> j...@nanthrax.net <mailto:j...@nanthrax.net>>>>> >>>>>>>>> napisał(a): >>>>>>>>>>>> Hi guys, >>>>>>>>>>>> >>>>>>>>>>>> In order to easily use Karaf in docker container, I created the >>>>>>>>> following >>>>>>>>>>>> PR: >>>>>>>>>>>> >>>>>>>>>>>> https://github.com/apache/karaf/pull/1203 < >>>> https://github.com/apache/karaf/pull/1203> < >>>>>>> https://github.com/apache/karaf/pull/1203 < >>>> https://github.com/apache/karaf/pull/1203>> < >>>>>>>>> https://github.com/apache/karaf/pull/1203 < >>>> https://github.com/apache/karaf/pull/1203> < >>>>>>> https://github.com/apache/karaf/pull/1203 < >>>> https://github.com/apache/karaf/pull/1203>>> < >>>>>>>>> https://github.com/apache/karaf/pull/1203 < >>>>>>> https://github.com/apache/karaf/pull/1203> < >>>>>>>>> https://github.com/apache/karaf/pull/1203 < >>>>>>> https://github.com/apache/karaf/pull/1203>>> < >>>>>>>>>>>> https://github.com/apache/karaf/pull/1203 < >>>>>>>>> https://github.com/apache/karaf/pull/1203> < >>>>>>>>> https://github.com/apache/karaf/pull/1203 < >>>>>>>>> https://github.com/apache/karaf/pull/1203>>> >>>>>>>>>>>> The purpose is allow use to override some configuration >> properties >>>>>>>>> using >>>>>>>>>>>> env variables. >>>>>>>>>>>> >>>>>>>>>>>> For instance, if you want to use 8102 instead of default 8101 >> ssh >>>>>>> port >>>>>>>>>>>> number, you can do: >>>>>>>>>>>> >>>>>>>>>>>> $ export KARAF_SSH_PORT=8102 >>>>>>>>>>>> $ bin/karaf >>>>>>>>>>>> >>>>>>>>>>>> I listed the environment variables here: >>>>>>>>>>>> >>>>>>>>>>>> >> https://github.com/apache/karaf/blob/4eda325e73d1d7dfbc0508258bde582e93d4f05e/manual/src/main/asciidoc/user-guide/configuration.adoc >>>> < >>>> >> https://github.com/apache/karaf/blob/4eda325e73d1d7dfbc0508258bde582e93d4f05e/manual/src/main/asciidoc/user-guide/configuration.adoc >>>>>>> < >>>>>>> >> https://github.com/apache/karaf/blob/4eda325e73d1d7dfbc0508258bde582e93d4f05e/manual/src/main/asciidoc/user-guide/configuration.adoc >>>>>>>>> < >>>>>>>>> >> https://github.com/apache/karaf/blob/4eda325e73d1d7dfbc0508258bde582e93d4f05e/manual/src/main/asciidoc/user-guide/configuration.adoc >>>>>>>>>>>> < >>>>>>>>>>>> >> https://github.com/apache/karaf/blob/4eda325e73d1d7dfbc0508258bde582e93d4f05e/manual/src/main/asciidoc/user-guide/configuration.adoc >>>>>>>>> < >>>>>>>>> >> https://github.com/apache/karaf/blob/4eda325e73d1d7dfbc0508258bde582e93d4f05e/manual/src/main/asciidoc/user-guide/configuration.adoc >>>>>>>>>>>> I didn’t define environment variables for all properties. If you >>>> see >>>>>>>>> some >>>>>>>>>>>> properties that it would be interesting to override by env >>>> variables, >>>>>>>>>>>> please let me know, I will update the PR. >>>>>>>>>>>> >>>>>>>>>>>> NB: I will also update pax* to use similar approach. >>>>>>>>>>>> >>>>>>>>>>>> Thoughts ? >>>>>>>>>>>> >>>>>>>>>>>> Regards >>>>>>>>>>>> JB >>>> >>
