You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tamaya.apache.org by Romain Manni-Bucau <rm...@gmail.com> on 2014/12/03 11:47:51 UTC

quick feedback before I forget

Hi

not basic discussion but opened the project today and wonder few
things. Trying to not forget I send a quick mail about it.

1) org.apache.tamaya.AggregationPolicy: why using functions?

    public static AggregationPolicy IGNORE_DUPLICATES() {
        return (k, v1, v2) -> v1 == null? v2 : v1;
    }

could be a constant:

    public static AggregationPolicy IGNORE_DUPLICATES = (k, v1, v2) ->
v1 == null? v2 : v1;

Seems more intuitive to me.


2) org.apache.tamaya.spi.ServiceProvider
Signature looks weird, for me it should be <T> T getService(Class<T>)
nothing more and Bootstrap shouldn't have any selection logic, the
provider has it if needed.

3) there are a lot of static final spi = loadSpi();
We should remove them and use an instance or just rely on the service
provider which could cache the instances but this design will be
broken pretty quickly in an environment (even a plain standalone app -
just try to use 2 SPIs for 2 parts of the app).

4) org.apache.tamaya.ConfigChangeSetBuilder#compare and few other
places: we'll surely want to do as in deltaspike ie prevent iteration
over some PropertyProviders (JNDI, DB, ...)

5) Stages: I'm not fully convinced using another Stage API brings
anything. Wonder if we shouldn't put it elsewhere like just using a
global prefix read from a SPI (default would be a system property) but
not making Stage feature explicit (for properties it doesn't bring
much much things).



Romain Manni-Bucau
@rmannibucau
http://www.tomitribe.com
http://rmannibucau.wordpress.com
https://github.com/rmannibucau

Re: quick feedback before I forget

Posted by Romain Manni-Bucau <rm...@gmail.com>.
3) I don't see how I use *my* implementation (of the SPI) if there are
2-3 available implementations.
4) not my point, I have an unknow set of keys in JNDI, I want to use
it when doing get("xxx") but I don't want you enumerate it. Think to
LDap for instance, do you want to browse it?


Romain Manni-Bucau
@rmannibucau
http://www.tomitribe.com
http://rmannibucau.wordpress.com
https://github.com/rmannibucau


2014-12-03 15:49 GMT+01:00 Tresch, Anatole <an...@credit-suisse.com>:
> 3) not sure if I get the point... some (but not all) SPIs may  have a default implementation, that is loaded ONLY, if nothing else is available from the Bootstrap (as a minimal fallback). This can be arguable, of course.
> 4) you can even mix things, e.g. for providers you can first use the service loader, e.g. to load the implementations also provided with the, and then CDI and then some more... I was more thinking on something similar as Instance in CDI, which also is terable... of course, if you provide a single value, iteration will only return one instance ;)
>
> Anatole
>
> -----Original Message-----
> From: Romain Manni-Bucau [mailto:rmannibucau@gmail.com]
> Sent: Mittwoch, 3. Dezember 2014 14:45
> To: dev@tamaya.incubator.apache.org
> Subject: Re: quick feedback before I forget
>
> 3) means default SPI is not a SPI but a default impl no?
> 4) if you use jndi to be able to read(key) you surely don't want to be
> able to iterate over it (and instantiate not expected instances for
> instance) so you'll basically prevent any diff with this data set.
> Maybe we need something like DataSet and ComparableDataSet (just the
> idea semantic is not great)
>
>
> Romain Manni-Bucau
> @rmannibucau
> http://www.tomitribe.com
> http://rmannibucau.wordpress.com
> https://github.com/rmannibucau
>
>
> 2014-12-03 14:40 GMT+01:00 Anatole Tresch <at...@gmail.com>:
>> Hi Romain
>>
>> 1) agree. Will fix it.
>> 2) agree. Will move logic.
>> 3) basically this still works, since you can choose in your singleton spi
>> impl whst is an appropriate strategy (eg contextual behsvour). Or the other
>> way round: what is your preferred solution?
>> 4) ???
>> 5) tbd yes, since we cann provide stage specific props as config ;) many
>> people like the idea having a Stage abdtraction... Other opinions ?
>>
>> Thanks for pointing this out!
>>
>> Anatole
>> Romain Manni-Bucau <rm...@gmail.com> schrieb am Mi., 3. Dez. 2014 um
>> 11:49:
>>
>>> Hi
>>>
>>> not basic discussion but opened the project today and wonder few
>>> things. Trying to not forget I send a quick mail about it.
>>>
>>> 1) org.apache.tamaya.AggregationPolicy: why using functions?
>>>
>>>     public static AggregationPolicy IGNORE_DUPLICATES() {
>>>         return (k, v1, v2) -> v1 == null? v2 : v1;
>>>     }
>>>
>>> could be a constant:
>>>
>>>     public static AggregationPolicy IGNORE_DUPLICATES = (k, v1, v2) ->
>>> v1 == null? v2 : v1;
>>>
>>> Seems more intuitive to me.
>>>
>>>
>>> 2) org.apache.tamaya.spi.ServiceProvider
>>> Signature looks weird, for me it should be <T> T getService(Class<T>)
>>> nothing more and Bootstrap shouldn't have any selection logic, the
>>> provider has it if needed.
>>>
>>> 3) there are a lot of static final spi = loadSpi();
>>> We should remove them and use an instance or just rely on the service
>>> provider which could cache the instances but this design will be
>>> broken pretty quickly in an environment (even a plain standalone app -
>>> just try to use 2 SPIs for 2 parts of the app).
>>>
>>> 4) org.apache.tamaya.ConfigChangeSetBuilder#compare and few other
>>> places: we'll surely want to do as in deltaspike ie prevent iteration
>>> over some PropertyProviders (JNDI, DB, ...)
>>>
>>> 5) Stages: I'm not fully convinced using another Stage API brings
>>> anything. Wonder if we shouldn't put it elsewhere like just using a
>>> global prefix read from a SPI (default would be a system property) but
>>> not making Stage feature explicit (for properties it doesn't bring
>>> much much things).
>>>
>>>
>>>
>>> Romain Manni-Bucau
>>> @rmannibucau
>>> http://www.tomitribe.com
>>> http://rmannibucau.wordpress.com
>>> https://github.com/rmannibucau
>>>

RE: quick feedback before I forget

Posted by "Tresch, Anatole " <an...@credit-suisse.com>.
3) not sure if I get the point... some (but not all) SPIs may  have a default implementation, that is loaded ONLY, if nothing else is available from the Bootstrap (as a minimal fallback). This can be arguable, of course.
4) you can even mix things, e.g. for providers you can first use the service loader, e.g. to load the implementations also provided with the, and then CDI and then some more... I was more thinking on something similar as Instance in CDI, which also is terable... of course, if you provide a single value, iteration will only return one instance ;)

Anatole

-----Original Message-----
From: Romain Manni-Bucau [mailto:rmannibucau@gmail.com] 
Sent: Mittwoch, 3. Dezember 2014 14:45
To: dev@tamaya.incubator.apache.org
Subject: Re: quick feedback before I forget

3) means default SPI is not a SPI but a default impl no?
4) if you use jndi to be able to read(key) you surely don't want to be
able to iterate over it (and instantiate not expected instances for
instance) so you'll basically prevent any diff with this data set.
Maybe we need something like DataSet and ComparableDataSet (just the
idea semantic is not great)


Romain Manni-Bucau
@rmannibucau
http://www.tomitribe.com
http://rmannibucau.wordpress.com
https://github.com/rmannibucau


2014-12-03 14:40 GMT+01:00 Anatole Tresch <at...@gmail.com>:
> Hi Romain
>
> 1) agree. Will fix it.
> 2) agree. Will move logic.
> 3) basically this still works, since you can choose in your singleton spi
> impl whst is an appropriate strategy (eg contextual behsvour). Or the other
> way round: what is your preferred solution?
> 4) ???
> 5) tbd yes, since we cann provide stage specific props as config ;) many
> people like the idea having a Stage abdtraction... Other opinions ?
>
> Thanks for pointing this out!
>
> Anatole
> Romain Manni-Bucau <rm...@gmail.com> schrieb am Mi., 3. Dez. 2014 um
> 11:49:
>
>> Hi
>>
>> not basic discussion but opened the project today and wonder few
>> things. Trying to not forget I send a quick mail about it.
>>
>> 1) org.apache.tamaya.AggregationPolicy: why using functions?
>>
>>     public static AggregationPolicy IGNORE_DUPLICATES() {
>>         return (k, v1, v2) -> v1 == null? v2 : v1;
>>     }
>>
>> could be a constant:
>>
>>     public static AggregationPolicy IGNORE_DUPLICATES = (k, v1, v2) ->
>> v1 == null? v2 : v1;
>>
>> Seems more intuitive to me.
>>
>>
>> 2) org.apache.tamaya.spi.ServiceProvider
>> Signature looks weird, for me it should be <T> T getService(Class<T>)
>> nothing more and Bootstrap shouldn't have any selection logic, the
>> provider has it if needed.
>>
>> 3) there are a lot of static final spi = loadSpi();
>> We should remove them and use an instance or just rely on the service
>> provider which could cache the instances but this design will be
>> broken pretty quickly in an environment (even a plain standalone app -
>> just try to use 2 SPIs for 2 parts of the app).
>>
>> 4) org.apache.tamaya.ConfigChangeSetBuilder#compare and few other
>> places: we'll surely want to do as in deltaspike ie prevent iteration
>> over some PropertyProviders (JNDI, DB, ...)
>>
>> 5) Stages: I'm not fully convinced using another Stage API brings
>> anything. Wonder if we shouldn't put it elsewhere like just using a
>> global prefix read from a SPI (default would be a system property) but
>> not making Stage feature explicit (for properties it doesn't bring
>> much much things).
>>
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau
>> http://www.tomitribe.com
>> http://rmannibucau.wordpress.com
>> https://github.com/rmannibucau
>>

Re: quick feedback before I forget

Posted by Romain Manni-Bucau <rm...@gmail.com>.
3) means default SPI is not a SPI but a default impl no?
4) if you use jndi to be able to read(key) you surely don't want to be
able to iterate over it (and instantiate not expected instances for
instance) so you'll basically prevent any diff with this data set.
Maybe we need something like DataSet and ComparableDataSet (just the
idea semantic is not great)


Romain Manni-Bucau
@rmannibucau
http://www.tomitribe.com
http://rmannibucau.wordpress.com
https://github.com/rmannibucau


2014-12-03 14:40 GMT+01:00 Anatole Tresch <at...@gmail.com>:
> Hi Romain
>
> 1) agree. Will fix it.
> 2) agree. Will move logic.
> 3) basically this still works, since you can choose in your singleton spi
> impl whst is an appropriate strategy (eg contextual behsvour). Or the other
> way round: what is your preferred solution?
> 4) ???
> 5) tbd yes, since we cann provide stage specific props as config ;) many
> people like the idea having a Stage abdtraction... Other opinions ?
>
> Thanks for pointing this out!
>
> Anatole
> Romain Manni-Bucau <rm...@gmail.com> schrieb am Mi., 3. Dez. 2014 um
> 11:49:
>
>> Hi
>>
>> not basic discussion but opened the project today and wonder few
>> things. Trying to not forget I send a quick mail about it.
>>
>> 1) org.apache.tamaya.AggregationPolicy: why using functions?
>>
>>     public static AggregationPolicy IGNORE_DUPLICATES() {
>>         return (k, v1, v2) -> v1 == null? v2 : v1;
>>     }
>>
>> could be a constant:
>>
>>     public static AggregationPolicy IGNORE_DUPLICATES = (k, v1, v2) ->
>> v1 == null? v2 : v1;
>>
>> Seems more intuitive to me.
>>
>>
>> 2) org.apache.tamaya.spi.ServiceProvider
>> Signature looks weird, for me it should be <T> T getService(Class<T>)
>> nothing more and Bootstrap shouldn't have any selection logic, the
>> provider has it if needed.
>>
>> 3) there are a lot of static final spi = loadSpi();
>> We should remove them and use an instance or just rely on the service
>> provider which could cache the instances but this design will be
>> broken pretty quickly in an environment (even a plain standalone app -
>> just try to use 2 SPIs for 2 parts of the app).
>>
>> 4) org.apache.tamaya.ConfigChangeSetBuilder#compare and few other
>> places: we'll surely want to do as in deltaspike ie prevent iteration
>> over some PropertyProviders (JNDI, DB, ...)
>>
>> 5) Stages: I'm not fully convinced using another Stage API brings
>> anything. Wonder if we shouldn't put it elsewhere like just using a
>> global prefix read from a SPI (default would be a system property) but
>> not making Stage feature explicit (for properties it doesn't bring
>> much much things).
>>
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau
>> http://www.tomitribe.com
>> http://rmannibucau.wordpress.com
>> https://github.com/rmannibucau
>>

Re: quick feedback before I forget

Posted by Anatole Tresch <at...@gmail.com>.
Hi Romain

1) agree. Will fix it.
2) agree. Will move logic.
3) basically this still works, since you can choose in your singleton spi
impl whst is an appropriate strategy (eg contextual behsvour). Or the other
way round: what is your preferred solution?
4) ???
5) tbd yes, since we cann provide stage specific props as config ;) many
people like the idea having a Stage abdtraction... Other opinions ?

Thanks for pointing this out!

Anatole
Romain Manni-Bucau <rm...@gmail.com> schrieb am Mi., 3. Dez. 2014 um
11:49:

> Hi
>
> not basic discussion but opened the project today and wonder few
> things. Trying to not forget I send a quick mail about it.
>
> 1) org.apache.tamaya.AggregationPolicy: why using functions?
>
>     public static AggregationPolicy IGNORE_DUPLICATES() {
>         return (k, v1, v2) -> v1 == null? v2 : v1;
>     }
>
> could be a constant:
>
>     public static AggregationPolicy IGNORE_DUPLICATES = (k, v1, v2) ->
> v1 == null? v2 : v1;
>
> Seems more intuitive to me.
>
>
> 2) org.apache.tamaya.spi.ServiceProvider
> Signature looks weird, for me it should be <T> T getService(Class<T>)
> nothing more and Bootstrap shouldn't have any selection logic, the
> provider has it if needed.
>
> 3) there are a lot of static final spi = loadSpi();
> We should remove them and use an instance or just rely on the service
> provider which could cache the instances but this design will be
> broken pretty quickly in an environment (even a plain standalone app -
> just try to use 2 SPIs for 2 parts of the app).
>
> 4) org.apache.tamaya.ConfigChangeSetBuilder#compare and few other
> places: we'll surely want to do as in deltaspike ie prevent iteration
> over some PropertyProviders (JNDI, DB, ...)
>
> 5) Stages: I'm not fully convinced using another Stage API brings
> anything. Wonder if we shouldn't put it elsewhere like just using a
> global prefix read from a SPI (default would be a system property) but
> not making Stage feature explicit (for properties it doesn't bring
> much much things).
>
>
>
> Romain Manni-Bucau
> @rmannibucau
> http://www.tomitribe.com
> http://rmannibucau.wordpress.com
> https://github.com/rmannibucau
>