geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <rmannibu...@gmail.com>
Subject Re: Planning to cut Config & Safeguard This Week
Date Tue, 02 Jan 2018 08:18:08 GMT
PS: pushed the system properties handling for the config


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>

2018-01-02 9:16 GMT+01:00 Romain Manni-Bucau <rmannibucau@gmail.com>:

> Here a few feedbacks:
>
> 1. org.apache.safeguard.api.config.ConfigFacade: it looks too risky and
> wrong as impl, setInstance should fail if already set + it should be per
> "app" (classloader with fallback on parent if not in the get? => how to
> clean? ServletContextListener for webapps?). If not it can only work as a
> lib embedded in an app and can't be industrialized as a container/envrt
> service
> 2. org.apache.safeguard.impl.config.MicroprofileAnnotationMapper#setInstance
> why?
> 3. org.apache.safeguard.impl.FailsafeExecutionManager#FailsafeExecutionManager()
> we should move the "instances" to cdi beans (@Inject?) and if null we do a
> plain new no? would avoid leaking (inter app) instances and old singletons
> spread accross the app and hard to change.
> 4. I updated the config facade to not enforce [config] dependency, can you
> review please?
> 5. in SPI files there are apache headers, it was common to not put them
> cause some impl were not supporting them well, do we want to strip them?
> 6. do we need to depend in failsafe lib? can't we do it ourself?
> 7. you mentionned it but org.apache.safeguard.impl.executionPlans.
> ExecutionPlanFactory#locateExecutionPlan(java.lang.reflect.Method) should
> be configurable and not a 5 threads pool, default should likely be a
> constant pool (same rule as 2 probably) otherwise more you use the lib more
> you have threads and can end up with an unbeliving system. In TomEE at the
> beginning we had that - with 1 thread - and saw systems with > 1000 threads
> doing almost nothing, we switched to 1 default pool with a few threads and
> system was as well behaving. Of course it must be customizable but default
> should be saner probably.
>
> Note linked to the impl: anyone knows why ConfigFacade is a class and not
> an interface? Abuse?
>
> What do you think?
>
>
>
> 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>
>
> 2018-01-02 8:50 GMT+01:00 Romain Manni-Bucau <rmannibucau@gmail.com>:
>
>> yes and no, what is true is a java 9 lib must have module SPI and
>> META-INF/services registration*s* but you also have optional imports so
>> this is still true. That said a fallback on system properties (hardcoded i
>> mean) works for me. Just don't want to enforce [config] to be here.
>>
>> Looking that now, will report soon
>>
>>
>> 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>
>>
>> 2018-01-01 22:59 GMT+01:00 John D. Ament <johndament@apache.org>:
>>
>>>
>>>
>>> On Mon, Jan 1, 2018 at 2:36 PM Romain Manni-Bucau <rmannibucau@gmail.com>
>>> wrote:
>>>
>>>> Yes, idea was to use config if there in right version and skip it with
>>>> an info log if not. Will try to check tmr. Thanks for the pointer.
>>>>
>>>
>>> No, it's not quite that. Honestly, with Java 9 and what not I'm a bit
>>> worried with that kind of approach since class importing is no longer
>>> behaving the same way.  I went with a ServiceLoader approach, this way even
>>> app servers can come up with their own configuration mechanism independent
>>> of MP.
>>>
>>>
>>>>
>>>> Le 1 janv. 2018 18:51, "John D. Ament" <johndament@apache.org> a
>>>> écrit :
>>>>
>>>>> You mean for safeguard?  If so its already there.  I do want to move
>>>>> it to a separate JAR so maybe OOTB we have a system property backed version?
>>>>>
>>>>> Take a look for ConfigFacade and MicroProfileConfigFacade.
>>>>>
>>>>> On Jan 1, 2018 12:37 PM, "Romain Manni-Bucau" <rmannibucau@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Any hope to have mp config optional before? Was planning to do it
>>>>>> before Xmas but didnt get a chance yet to code it. Can try later
this week
>>>>>> probably.
>>>>>>
>>>>>> Le 1 janv. 2018 17:19, "John D. Ament" <johndament@apache.org>
a
>>>>>> écrit :
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Jan 1, 2018 at 11:10 AM Mark Struberg <struberg@yahoo.de>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> +1 go for it!
>>>>>>>>
>>>>>>>> > Safeguard requires a Config 1.2 implementation to run,
since
>>>>>>>> Config 1.2
>>>>>>>>
>>>>>>>> Geronimo-config-1.1  is microprofile-config 1.2, right?
>>>>>>>>
>>>>>>>
>>>>>>> Yes.  Between the bugs found in the impl and the spec issues
I saw,
>>>>>>> GConfig 1.0 ended up implementing MP Config 1.1.  I think only
IBM shipped
>>>>>>> an impl of just Config 1.0.
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> LieGrue,
>>>>>>>> strub
>>>>>>>>
>>>>>>>> > Am 01.01.2018 um 16:34 schrieb John D. Ament <
>>>>>>>> johndament@apache.org>:
>>>>>>>> >
>>>>>>>> > Hey guys
>>>>>>>> >
>>>>>>>> > Just pushed up the last of the changes for Safeguard
to make it
>>>>>>>> pass Fault Tolerance 1.0's TCK.  There's a small change I
still want to
>>>>>>>> make it to allow the executor to be pluggable, and plan to
have a following
>>>>>>>> release soon that introduces more configurable properties.
>>>>>>>> >
>>>>>>>> > With that said, I'm going to plan to stage the Config
1.1 release
>>>>>>>> tomorrow and start testing the Safeguard release process
(since this'll be
>>>>>>>> the first time we're releasing a git repo).  Once that's
working, I'll plan
>>>>>>>> to stage that release as well.
>>>>>>>> >
>>>>>>>> > Please note - Safeguard requires a Config 1.2 implementation
to
>>>>>>>> run, since Config 1.2 introduces common sense converters
(for enums in
>>>>>>>> particular) and Class converter built in.  I didn't want
to register a
>>>>>>>> custom converter.
>>>>>>>> >
>>>>>>>> > John
>>>>>>>>
>>>>>>>>
>>
>

Mime
View raw message