Re: Renaming Geronimo Config?

[Romain Manni-Bucau]

I wouldnt go with xbean. Why not naming it if you dont want of G?

Concretely there are 2 options:

- keep G and promote the project with its new goal
- drop it and name it with something new

Not a fan of last one since ultimately it means we must drop the name in
the project and is not consistent with last months discussions IMHO.

Wdyt?

Le 9 août 2017 02:33, "John D. Ament"  a écrit :

> Not to stir that pot, but does it make sense to just rename Geronimo
> itself to XBean?
>
> I'm assuming then for config you're talking about changing the coordinates
> to org.apache.xbean:xbean-config(-impl) ?
>
> John
>
> On Tue, Aug 8, 2017 at 7:15 PM Mark Struberg  wrote:
>
>> Perfectly fine for me. I'd still give it a different release lifecycle
>> from the rest of xbean.
>> Actually it makes not much sense for the rest of xbean to share the same
>> version.
>> Most of the components do not have any common ground with each other.
>>
>> LieGrue,
>> strub
>>
>>
>> > Am 09.08.2017 um 01:11 schrieb David Blevins :
>> >
>> > Can we rename Geronimo Config?  I think the name is strongly stuck with
>> the app server.  From experience in EJB land, try to repurpose names is
>> usually an uphill battle.
>> >
>> > If we wanted to go with the grain, we could call it XBean Config.  Open
>> to other names as well.
>> >
>> > If we did call it XBean Config, I’m not sure there’s a need to have the
>> same version as the other xbean components.  We could, but I think 1.0
>> would still be fine.
>> >
>> >
>> > --
>> > David Blevins
>> > http://twitter.com/dblevins
>> > http://www.tomitribe.com
>> >
>>
>>

Re: svn commit: r1804397 - in /geronimo/components/config/trunk: ./ impl/ impl/src/main/java/org/apache/geronimo/config/cdi/ impl/src/test/java/org/apache/geronimo/config/test/internal/

[John D. Ament]

Ok, I've readded some of the Weld3 support, 3 tests are failing.

Some interesting notes:

- If I remove Provider support, it generally works.  However, I suddenly
get duplicate beans of type Provider, which is confusing (but probably
because of the alternative flag).
- I can get everything to pass in Weld, and 1 test failure in in OWB with
the following:

- Set alternative = false
- Remove Provider from the types
- Remove the filter on types to check if class.
- Change getId to use all of the types (makes a better toString)


If we can figure out the alternative issue with OWB, we'll be good to go
again.

John

On Tue, Aug 8, 2017 at 8:44 AM John D. Ament  wrote:

> I can get to some of it.
>
> We have a couple of issues now though:
>
> https://issues.jboss.org/browse/WELD-2411
> https://issues.jboss.org/browse/CDI-712
>
> Martin doesn't feel the spec allows you to override Provider impl's.
> Please feel free to weigh in :-)
>
> John
>
>
> On Tue, Aug 8, 2017 at 7:33 AM Mark Struberg  wrote:
>
>> Oh I see what you mean.
>> Do you mind to re-apply your fix or should I?
>>
>> Btw, I also tested with the -PWeld3 but I was not able to resolve
>> arquillian-weld-embedded:jar:2.0.0-SNAPSHOT
>> We might need to add a  for it in the Weld3 profile.
>>
>> I also would love to add a Weld profile. I thought we have this already
>> but seems we missed it?
>>
>> txs and LieGrue,
>> strub
>>
>> > Am 08.08.2017 um 12:58 schrieb John D. Ament :
>> >
>> > One note - hardcoding "isAlternative" to true breaks in Weld.  Unless
>> there is a base bean of same type, the alternative is ignored.  Hence why I
>> only did isAlternative on the provider type, all others are just regular
>> beans.
>> >
>> > John
>> >
>> > On Tue, Aug 8, 2017 at 6:55 AM  wrote:
>> > Author: struberg
>> > Date: Tue Aug  8 10:55:56 2017
>> > New Revision: 1804397
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1804397=rev
>> > Log:
>> > GERONIMO-6577 move back to a more dynamic version
>> >
>> > The goal of r1800748 to calculate all information upfront could not be
>> achieved
>> > so we move back to the old version
>> >
>> > Modified:
>> > geronimo/components/config/trunk/impl/debug-suite.xml
>> > geronimo/components/config/trunk/impl/pom.xml
>> >
>>  
>> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
>> >
>>  
>> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigInjectionBean.java
>> >
>>  
>> geronimo/components/config/trunk/impl/src/test/java/org/apache/geronimo/config/test/internal/ProviderTest.java
>> > geronimo/components/config/trunk/pom.xml
>> >
>> > Modified: geronimo/components/config/trunk/impl/debug-suite.xml
>> > URL:
>> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/debug-suite.xml?rev=1804397=1804396=1804397=diff
>> >
>> ==
>> > --- geronimo/components/config/trunk/impl/debug-suite.xml (original)
>> > +++ geronimo/components/config/trunk/impl/debug-suite.xml Tue Aug  8
>> 10:55:56 2017
>> > @@ -24,7 +24,7 @@
>> >  
>> >  
>> >  
>> > -> name="org.eclipse.microprofile.config.tck.CdiOptionalInjectionTest">
>> > +> name="org.eclipse.microprofile.config.tck.CDIPlainInjectionTest">
>> >  
>> >  
>> >  
>> >
>> > Modified: geronimo/components/config/trunk/impl/pom.xml
>> > URL:
>> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/pom.xml?rev=1804397=1804396=1804397=diff
>> >
>> ==
>> > --- geronimo/components/config/trunk/impl/pom.xml (original)
>> > +++ geronimo/components/config/trunk/impl/pom.xml Tue Aug  8 10:55:56
>> 2017
>> > @@ -27,7 +27,6 @@
>> >  
>> >
>> >  geronimo-config-impl
>> > -Geronimo Microprofile Configuration :: Implementation
>> >
>> >  
>> >  
>> >
>> > Modified:
>> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
>> > URL:
>> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java?rev=1804397=1804396=1804397=diff
>> >
>> ==
>> > ---
>> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
>> (original)
>> > +++
>> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
>> Tue Aug  8 10:55:56 2017
>> > @@ -16,54 +16,38 @@
>> >   */
>> >  package org.apache.geronimo.config.cdi;
>> >
>> > -import org.apache.geronimo.config.ConfigImpl;
>> > -import org.eclipse.microprofile.config.Config;
>> > -import 

Re: Renaming Geronimo Config?

[John D. Ament]

Not to stir that pot, but does it make sense to just rename Geronimo itself
to XBean?

I'm assuming then for config you're talking about changing the coordinates
to org.apache.xbean:xbean-config(-impl) ?

John

On Tue, Aug 8, 2017 at 7:15 PM Mark Struberg  wrote:

> Perfectly fine for me. I'd still give it a different release lifecycle
> from the rest of xbean.
> Actually it makes not much sense for the rest of xbean to share the same
> version.
> Most of the components do not have any common ground with each other.
>
> LieGrue,
> strub
>
>
> > Am 09.08.2017 um 01:11 schrieb David Blevins :
> >
> > Can we rename Geronimo Config?  I think the name is strongly stuck with
> the app server.  From experience in EJB land, try to repurpose names is
> usually an uphill battle.
> >
> > If we wanted to go with the grain, we could call it XBean Config.  Open
> to other names as well.
> >
> > If we did call it XBean Config, I’m not sure there’s a need to have the
> same version as the other xbean components.  We could, but I think 1.0
> would still be fine.
> >
> >
> > --
> > David Blevins
> > http://twitter.com/dblevins
> > http://www.tomitribe.com
> >
>
>

Re: Renaming Geronimo Config?

[Mark Struberg]

Perfectly fine for me. I'd still give it a different release lifecycle from the 
rest of xbean.
Actually it makes not much sense for the rest of xbean to share the same 
version.
Most of the components do not have any common ground with each other.

LieGrue,
strub

 
> Am 09.08.2017 um 01:11 schrieb David Blevins :
> 
> Can we rename Geronimo Config?  I think the name is strongly stuck with the 
> app server.  From experience in EJB land, try to repurpose names is usually 
> an uphill battle.
> 
> If we wanted to go with the grain, we could call it XBean Config.  Open to 
> other names as well.
> 
> If we did call it XBean Config, I’m not sure there’s a need to have the same 
> version as the other xbean components.  We could, but I think 1.0 would still 
> be fine.
> 
> 
> -- 
> David Blevins
> http://twitter.com/dblevins
> http://www.tomitribe.com
> 

Renaming Geronimo Config?

[David Blevins]

Can we rename Geronimo Config?  I think the name is strongly stuck with the app 
server.  From experience in EJB land, try to repurpose names is usually an 
uphill battle.

If we wanted to go with the grain, we could call it XBean Config.  Open to 
other names as well.

If we did call it XBean Config, I’m not sure there’s a need to have the same 
version as the other xbean components.  We could, but I think 1.0 would still 
be fine.


-- 
David Blevins
http://twitter.com/dblevins
http://www.tomitribe.com

Re: [VOTE] Release Apache Geronimo-config 1.0

[Mark Struberg]

John, I think the produced Config instance is @ApplicationScoped, isn't?

So serialisability is guaranteed by the CDI NormalScope proxy.

LieGrue,
strub

> Am 08.08.2017 um 13:03 schrieb John D. Ament :
> 
> On the subject of serializable.  We fail one of the comments in the MP Spec 
> (but not in the TCK!) our injected Config class is *not* serializable. But 
> for that to work, all config sources and all converters must be serializable.
> 

Re: svn commit: r1804397 - in /geronimo/components/config/trunk: ./ impl/ impl/src/main/java/org/apache/geronimo/config/cdi/ impl/src/test/java/org/apache/geronimo/config/test/internal/

[John D. Ament]

I can get to some of it.

We have a couple of issues now though:

https://issues.jboss.org/browse/WELD-2411
https://issues.jboss.org/browse/CDI-712

Martin doesn't feel the spec allows you to override Provider impl's.
Please feel free to weigh in :-)

John

On Tue, Aug 8, 2017 at 7:33 AM Mark Struberg  wrote:

> Oh I see what you mean.
> Do you mind to re-apply your fix or should I?
>
> Btw, I also tested with the -PWeld3 but I was not able to resolve
> arquillian-weld-embedded:jar:2.0.0-SNAPSHOT
> We might need to add a  for it in the Weld3 profile.
>
> I also would love to add a Weld profile. I thought we have this already
> but seems we missed it?
>
> txs and LieGrue,
> strub
>
> > Am 08.08.2017 um 12:58 schrieb John D. Ament :
> >
> > One note - hardcoding "isAlternative" to true breaks in Weld.  Unless
> there is a base bean of same type, the alternative is ignored.  Hence why I
> only did isAlternative on the provider type, all others are just regular
> beans.
> >
> > John
> >
> > On Tue, Aug 8, 2017 at 6:55 AM  wrote:
> > Author: struberg
> > Date: Tue Aug  8 10:55:56 2017
> > New Revision: 1804397
> >
> > URL: http://svn.apache.org/viewvc?rev=1804397=rev
> > Log:
> > GERONIMO-6577 move back to a more dynamic version
> >
> > The goal of r1800748 to calculate all information upfront could not be
> achieved
> > so we move back to the old version
> >
> > Modified:
> > geronimo/components/config/trunk/impl/debug-suite.xml
> > geronimo/components/config/trunk/impl/pom.xml
> >
>  
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
> >
>  
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigInjectionBean.java
> >
>  
> geronimo/components/config/trunk/impl/src/test/java/org/apache/geronimo/config/test/internal/ProviderTest.java
> > geronimo/components/config/trunk/pom.xml
> >
> > Modified: geronimo/components/config/trunk/impl/debug-suite.xml
> > URL:
> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/debug-suite.xml?rev=1804397=1804396=1804397=diff
> >
> ==
> > --- geronimo/components/config/trunk/impl/debug-suite.xml (original)
> > +++ geronimo/components/config/trunk/impl/debug-suite.xml Tue Aug  8
> 10:55:56 2017
> > @@ -24,7 +24,7 @@
> >  
> >  
> >  
> > - name="org.eclipse.microprofile.config.tck.CdiOptionalInjectionTest">
> > + name="org.eclipse.microprofile.config.tck.CDIPlainInjectionTest">
> >  
> >  
> >  
> >
> > Modified: geronimo/components/config/trunk/impl/pom.xml
> > URL:
> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/pom.xml?rev=1804397=1804396=1804397=diff
> >
> ==
> > --- geronimo/components/config/trunk/impl/pom.xml (original)
> > +++ geronimo/components/config/trunk/impl/pom.xml Tue Aug  8 10:55:56
> 2017
> > @@ -27,7 +27,6 @@
> >  
> >
> >  geronimo-config-impl
> > -Geronimo Microprofile Configuration :: Implementation
> >
> >  
> >  
> >
> > Modified:
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
> > URL:
> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java?rev=1804397=1804396=1804397=diff
> >
> ==
> > ---
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
> (original)
> > +++
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
> Tue Aug  8 10:55:56 2017
> > @@ -16,54 +16,38 @@
> >   */
> >  package org.apache.geronimo.config.cdi;
> >
> > -import org.apache.geronimo.config.ConfigImpl;
> > -import org.eclipse.microprofile.config.Config;
> > -import org.eclipse.microprofile.config.inject.ConfigProperty;
> > -import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
> > +import java.lang.reflect.Type;
> > +import java.util.ArrayList;
> > +import java.util.HashMap;
> > +import java.util.HashSet;
> > +import java.util.List;
> > +import java.util.Map;
> > +import java.util.Optional;
> > +import java.util.Set;
> > +import java.util.stream.Collectors;
> >
> > -import javax.enterprise.context.spi.CreationalContext;
> >  import javax.enterprise.event.Observes;
> >  import javax.enterprise.inject.spi.AfterBeanDiscovery;
> >  import javax.enterprise.inject.spi.AfterDeploymentValidation;
> > -import javax.enterprise.inject.spi.AnnotatedMember;
> > -import javax.enterprise.inject.spi.AnnotatedType;
> >  import javax.enterprise.inject.spi.BeanManager;
> > -import javax.enterprise.inject.spi.BeforeBeanDiscovery;
> 

Re: svn commit: r1804397 - in /geronimo/components/config/trunk: ./ impl/ impl/src/main/java/org/apache/geronimo/config/cdi/ impl/src/test/java/org/apache/geronimo/config/test/internal/

[Mark Struberg]

Oh I see what you mean. 
Do you mind to re-apply your fix or should I?

Btw, I also tested with the -PWeld3 but I was not able to resolve 
arquillian-weld-embedded:jar:2.0.0-SNAPSHOT
We might need to add a  for it in the Weld3 profile.

I also would love to add a Weld profile. I thought we have this already but 
seems we missed it?

txs and LieGrue,
strub

> Am 08.08.2017 um 12:58 schrieb John D. Ament :
> 
> One note - hardcoding "isAlternative" to true breaks in Weld.  Unless there 
> is a base bean of same type, the alternative is ignored.  Hence why I only 
> did isAlternative on the provider type, all others are just regular beans.
> 
> John
> 
> On Tue, Aug 8, 2017 at 6:55 AM  wrote:
> Author: struberg
> Date: Tue Aug  8 10:55:56 2017
> New Revision: 1804397
> 
> URL: http://svn.apache.org/viewvc?rev=1804397=rev
> Log:
> GERONIMO-6577 move back to a more dynamic version
> 
> The goal of r1800748 to calculate all information upfront could not be 
> achieved
> so we move back to the old version
> 
> Modified:
> geronimo/components/config/trunk/impl/debug-suite.xml
> geronimo/components/config/trunk/impl/pom.xml
> 
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
> 
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigInjectionBean.java
> 
> geronimo/components/config/trunk/impl/src/test/java/org/apache/geronimo/config/test/internal/ProviderTest.java
> geronimo/components/config/trunk/pom.xml
> 
> Modified: geronimo/components/config/trunk/impl/debug-suite.xml
> URL: 
> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/debug-suite.xml?rev=1804397=1804396=1804397=diff
> ==
> --- geronimo/components/config/trunk/impl/debug-suite.xml (original)
> +++ geronimo/components/config/trunk/impl/debug-suite.xml Tue Aug  8 10:55:56 
> 2017
> @@ -24,7 +24,7 @@
>  
>  
>  
> - name="org.eclipse.microprofile.config.tck.CdiOptionalInjectionTest">
> + name="org.eclipse.microprofile.config.tck.CDIPlainInjectionTest">
>  
>  
>  
> 
> Modified: geronimo/components/config/trunk/impl/pom.xml
> URL: 
> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/pom.xml?rev=1804397=1804396=1804397=diff
> ==
> --- geronimo/components/config/trunk/impl/pom.xml (original)
> +++ geronimo/components/config/trunk/impl/pom.xml Tue Aug  8 10:55:56 2017
> @@ -27,7 +27,6 @@
>  
> 
>  geronimo-config-impl
> -Geronimo Microprofile Configuration :: Implementation
> 
>  
>  
> 
> Modified: 
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
> URL: 
> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java?rev=1804397=1804396=1804397=diff
> ==
> --- 
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
>  (original)
> +++ 
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
>  Tue Aug  8 10:55:56 2017
> @@ -16,54 +16,38 @@
>   */
>  package org.apache.geronimo.config.cdi;
> 
> -import org.apache.geronimo.config.ConfigImpl;
> -import org.eclipse.microprofile.config.Config;
> -import org.eclipse.microprofile.config.inject.ConfigProperty;
> -import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
> +import java.lang.reflect.Type;
> +import java.util.ArrayList;
> +import java.util.HashMap;
> +import java.util.HashSet;
> +import java.util.List;
> +import java.util.Map;
> +import java.util.Optional;
> +import java.util.Set;
> +import java.util.stream.Collectors;
> 
> -import javax.enterprise.context.spi.CreationalContext;
>  import javax.enterprise.event.Observes;
>  import javax.enterprise.inject.spi.AfterBeanDiscovery;
>  import javax.enterprise.inject.spi.AfterDeploymentValidation;
> -import javax.enterprise.inject.spi.AnnotatedMember;
> -import javax.enterprise.inject.spi.AnnotatedType;
>  import javax.enterprise.inject.spi.BeanManager;
> -import javax.enterprise.inject.spi.BeforeBeanDiscovery;
>  import javax.enterprise.inject.spi.BeforeShutdown;
> +import javax.enterprise.inject.spi.DeploymentException;
>  import javax.enterprise.inject.spi.Extension;
>  import javax.enterprise.inject.spi.InjectionPoint;
>  import javax.enterprise.inject.spi.ProcessInjectionPoint;
>  import javax.inject.Provider;
> -import java.lang.reflect.ParameterizedType;
> -import java.lang.reflect.Type;
> -import java.util.ArrayList;
> -import java.util.Arrays;
> -import java.util.Collection;
> -import 

Re: [VOTE] Release Apache Geronimo-config 1.0

[Mark Struberg]

Only if you upcast and for _some_ Lambdas.
Lambdas can create multiple different forms of bytecode.

In our case it did create an inner class of the Extension which then still gets 
referenced as $0. 
So even if the Lambda implements Serializable it still references back to the 
Extension which is not Serializable.

LieGrue,
strub


> Am 08.08.2017 um 13:06 schrieb Romain Manni-Bucau :
> 
> @Mark: lambda are serializable (java.lang.invoke.SerializedLambda)
> 
> 
> Romain Manni-Bucau
> @rmannibucau |  Blog | Old Blog | Github | LinkedIn | JavaEE Factory
> 
> 2017-08-08 13:03 GMT+02:00 John D. Ament :
> On the subject of serializable.  We fail one of the comments in the MP Spec 
> (but not in the TCK!) our injected Config class is *not* serializable. But 
> for that to work, all config sources and all converters must be serializable.
> 
> 
> On Tue, Aug 8, 2017 at 6:57 AM Mark Struberg  wrote:
> Another thing I noticed:
> 
> While using a Lambda as impl for Provider is very elegant, it still misses an 
> important functionality: it is not Serializable, and thus injecting a 
> @ConfigProperty Provider into a @SessionScoped bean would blow up at 
> runtime.
> 
> 
> LieGrue,
> strub
> 
> 
> > Am 08.08.2017 um 12:35 schrieb Mark Struberg :
> >
> > I fear that's exactly not possible. CDI resolves with type + qualifier. 
> > Since the type is not unique (mulitple Provider) you would need to 
> > make the Qualifier unique. Means for each of them you need to dynamically 
> > create a new binding ConfigProperty Implementation.
> > There are 2 problems:
> > a.) I have no clue how you can dynamically remove the @Nonbinding from the 
> > ConfigProperty annotation.
> > b.) you have to register tons of such qualifiers, which makes the whole 
> > resolving slower again. You just moved the effort from the Bean#create to 
> > BeanManager#getBeans, that's all
> >
> > LieGrue,
> > strub
> >
> >> Am 08.08.2017 um 12:16 schrieb Romain Manni-Bucau :
> >>
> >> b.) the default naming (name="") would break it anyway. You again would 
> >> need to create dynamic qualifiers for each of those injection points.
> >>
> >> Not true since you handle it in the extension precomputing it
> >>
> >
> 
> 

Re: [VOTE] Release Apache Geronimo-config 1.0

[Romain Manni-Bucau]

@Mark: lambda are serializable (java.lang.invoke.SerializedLambda)


Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | JavaEE Factory


2017-08-08 13:03 GMT+02:00 John D. Ament :

> On the subject of serializable.  We fail one of the comments in the MP
> Spec (but not in the TCK!) our injected Config class is *not* serializable.
> But for that to work, all config sources and all converters must be
> serializable.
>
>
> On Tue, Aug 8, 2017 at 6:57 AM Mark Struberg  wrote:
>
>> Another thing I noticed:
>>
>> While using a Lambda as impl for Provider is very elegant, it still
>> misses an important functionality: it is not Serializable, and thus
>> injecting a @ConfigProperty Provider into a @SessionScoped bean
>> would blow up at runtime.
>>
>>
>> LieGrue,
>> strub
>>
>>
>> > Am 08.08.2017 um 12:35 schrieb Mark Struberg :
>> >
>> > I fear that's exactly not possible. CDI resolves with type + qualifier.
>> Since the type is not unique (mulitple Provider) you would need to
>> make the Qualifier unique. Means for each of them you need to dynamically
>> create a new binding ConfigProperty Implementation.
>> > There are 2 problems:
>> > a.) I have no clue how you can dynamically remove the @Nonbinding from
>> the ConfigProperty annotation.
>> > b.) you have to register tons of such qualifiers, which makes the whole
>> resolving slower again. You just moved the effort from the Bean#create to
>> BeanManager#getBeans, that's all
>> >
>> > LieGrue,
>> > strub
>> >
>> >> Am 08.08.2017 um 12:16 schrieb Romain Manni-Bucau <
>> rmannibu...@gmail.com>:
>> >>
>> >> b.) the default naming (name="") would break it anyway. You again
>> would need to create dynamic qualifiers for each of those injection points.
>> >>
>> >> Not true since you handle it in the extension precomputing it
>> >>
>> >
>>
>>

Re: [VOTE] Release Apache Geronimo-config 1.0

[John D. Ament]

On the subject of serializable.  We fail one of the comments in the MP Spec
(but not in the TCK!) our injected Config class is *not* serializable. But
for that to work, all config sources and all converters must be
serializable.

On Tue, Aug 8, 2017 at 6:57 AM Mark Struberg  wrote:

> Another thing I noticed:
>
> While using a Lambda as impl for Provider is very elegant, it still misses
> an important functionality: it is not Serializable, and thus injecting a
> @ConfigProperty Provider into a @SessionScoped bean would blow up
> at runtime.
>
>
> LieGrue,
> strub
>
>
> > Am 08.08.2017 um 12:35 schrieb Mark Struberg :
> >
> > I fear that's exactly not possible. CDI resolves with type + qualifier.
> Since the type is not unique (mulitple Provider) you would need to
> make the Qualifier unique. Means for each of them you need to dynamically
> create a new binding ConfigProperty Implementation.
> > There are 2 problems:
> > a.) I have no clue how you can dynamically remove the @Nonbinding from
> the ConfigProperty annotation.
> > b.) you have to register tons of such qualifiers, which makes the whole
> resolving slower again. You just moved the effort from the Bean#create to
> BeanManager#getBeans, that's all
> >
> > LieGrue,
> > strub
> >
> >> Am 08.08.2017 um 12:16 schrieb Romain Manni-Bucau <
> rmannibu...@gmail.com>:
> >>
> >> b.) the default naming (name="") would break it anyway. You again would
> need to create dynamic qualifiers for each of those injection points.
> >>
> >> Not true since you handle it in the extension precomputing it
> >>
> >
>
>

Re: svn commit: r1804397 - in /geronimo/components/config/trunk: ./ impl/ impl/src/main/java/org/apache/geronimo/config/cdi/ impl/src/test/java/org/apache/geronimo/config/test/internal/

[John D. Ament]

One note - hardcoding "isAlternative" to true breaks in Weld.  Unless there
is a base bean of same type, the alternative is ignored.  Hence why I only
did isAlternative on the provider type, all others are just regular beans.

John

On Tue, Aug 8, 2017 at 6:55 AM  wrote:

> Author: struberg
> Date: Tue Aug  8 10:55:56 2017
> New Revision: 1804397
>
> URL: http://svn.apache.org/viewvc?rev=1804397=rev
> Log:
> GERONIMO-6577 move back to a more dynamic version
>
> The goal of r1800748 to calculate all information upfront could not be
> achieved
> so we move back to the old version
>
> Modified:
> geronimo/components/config/trunk/impl/debug-suite.xml
> geronimo/components/config/trunk/impl/pom.xml
>
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
>
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigInjectionBean.java
>
> geronimo/components/config/trunk/impl/src/test/java/org/apache/geronimo/config/test/internal/ProviderTest.java
> geronimo/components/config/trunk/pom.xml
>
> Modified: geronimo/components/config/trunk/impl/debug-suite.xml
> URL:
> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/debug-suite.xml?rev=1804397=1804396=1804397=diff
>
> ==
> --- geronimo/components/config/trunk/impl/debug-suite.xml (original)
> +++ geronimo/components/config/trunk/impl/debug-suite.xml Tue Aug  8
> 10:55:56 2017
> @@ -24,7 +24,7 @@
>  
>  
>  
> - name="org.eclipse.microprofile.config.tck.CdiOptionalInjectionTest">
> + name="org.eclipse.microprofile.config.tck.CDIPlainInjectionTest">
>  
>  
>  
>
> Modified: geronimo/components/config/trunk/impl/pom.xml
> URL:
> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/pom.xml?rev=1804397=1804396=1804397=diff
>
> ==
> --- geronimo/components/config/trunk/impl/pom.xml (original)
> +++ geronimo/components/config/trunk/impl/pom.xml Tue Aug  8 10:55:56 2017
> @@ -27,7 +27,6 @@
>  
>
>  geronimo-config-impl
> -Geronimo Microprofile Configuration :: Implementation
>
>  
>  
>
> Modified:
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
> URL:
> http://svn.apache.org/viewvc/geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java?rev=1804397=1804396=1804397=diff
>
> ==
> ---
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
> (original)
> +++
> geronimo/components/config/trunk/impl/src/main/java/org/apache/geronimo/config/cdi/ConfigExtension.java
> Tue Aug  8 10:55:56 2017
> @@ -16,54 +16,38 @@
>   */
>  package org.apache.geronimo.config.cdi;
>
> -import org.apache.geronimo.config.ConfigImpl;
> -import org.eclipse.microprofile.config.Config;
> -import org.eclipse.microprofile.config.inject.ConfigProperty;
> -import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
> +import java.lang.reflect.Type;
> +import java.util.ArrayList;
> +import java.util.HashMap;
> +import java.util.HashSet;
> +import java.util.List;
> +import java.util.Map;
> +import java.util.Optional;
> +import java.util.Set;
> +import java.util.stream.Collectors;
>
> -import javax.enterprise.context.spi.CreationalContext;
>  import javax.enterprise.event.Observes;
>  import javax.enterprise.inject.spi.AfterBeanDiscovery;
>  import javax.enterprise.inject.spi.AfterDeploymentValidation;
> -import javax.enterprise.inject.spi.AnnotatedMember;
> -import javax.enterprise.inject.spi.AnnotatedType;
>  import javax.enterprise.inject.spi.BeanManager;
> -import javax.enterprise.inject.spi.BeforeBeanDiscovery;
>  import javax.enterprise.inject.spi.BeforeShutdown;
> +import javax.enterprise.inject.spi.DeploymentException;
>  import javax.enterprise.inject.spi.Extension;
>  import javax.enterprise.inject.spi.InjectionPoint;
>  import javax.enterprise.inject.spi.ProcessInjectionPoint;
>  import javax.inject.Provider;
> -import java.lang.reflect.ParameterizedType;
> -import java.lang.reflect.Type;
> -import java.util.ArrayList;
> -import java.util.Arrays;
> -import java.util.Collection;
> -import java.util.HashMap;
> -import java.util.HashSet;
> -import java.util.List;
> -import java.util.Map;
> -import java.util.Objects;
> -import java.util.Optional;
> -import java.util.Set;
> -import java.util.function.BiFunction;
> -import java.util.stream.Stream;
>
> -import static java.util.function.Function.identity;
> -import static java.util.stream.Collectors.toMap;
> -import static org.eclipse.microprofile.config.ConfigProvider.getConfig;
> +import 

Re: [VOTE] Release Apache Geronimo-config 1.0

[Mark Struberg]

Another thing I noticed:

While using a Lambda as impl for Provider is very elegant, it still misses an 
important functionality: it is not Serializable, and thus injecting a 
@ConfigProperty Provider into a @SessionScoped bean would blow up at 
runtime.


LieGrue,
strub


> Am 08.08.2017 um 12:35 schrieb Mark Struberg :
> 
> I fear that's exactly not possible. CDI resolves with type + qualifier. Since 
> the type is not unique (mulitple Provider) you would need to make the 
> Qualifier unique. Means for each of them you need to dynamically create a new 
> binding ConfigProperty Implementation. 
> There are 2 problems:
> a.) I have no clue how you can dynamically remove the @Nonbinding from the 
> ConfigProperty annotation. 
> b.) you have to register tons of such qualifiers, which makes the whole 
> resolving slower again. You just moved the effort from the Bean#create to 
> BeanManager#getBeans, that's all
> 
> LieGrue,
> strub
> 
>> Am 08.08.2017 um 12:16 schrieb Romain Manni-Bucau :
>> 
>> b.) the default naming (name="") would break it anyway. You again would need 
>> to create dynamic qualifiers for each of those injection points.
>> 
>> Not true since you handle it in the extension precomputing it
>> 
> 

Re: [VOTE] Release Apache Geronimo-config 1.0

[Mark Struberg]

I fear that's exactly not possible. CDI resolves with type + qualifier. Since 
the type is not unique (mulitple Provider) you would need to make the 
Qualifier unique. Means for each of them you need to dynamically create a new 
binding ConfigProperty Implementation. 
There are 2 problems:
a.) I have no clue how you can dynamically remove the @Nonbinding from the 
ConfigProperty annotation. 
b.) you have to register tons of such qualifiers, which makes the whole 
resolving slower again. You just moved the effort from the Bean#create to 
BeanManager#getBeans, that's all

LieGrue,
strub

> Am 08.08.2017 um 12:16 schrieb Romain Manni-Bucau :
> 
> b.) the default naming (name="") would break it anyway. You again would need 
> to create dynamic qualifiers for each of those injection points.
> 
> Not true since you handle it in the extension precomputing it
> 

Re: [VOTE] Release Apache Geronimo-config 1.0

[Romain Manni-Bucau]

2017-08-08 11:40 GMT+02:00 Mark Struberg :

> I think the API is quite fine. What are your concrete concerns?
>
> There are btw 2 reasons why the name in the @ConfigProperty is @Nonbinding:
>
> a.) you would need to register about 2000 different Beans otherwise (one
> per injection point basically
>

yep but without much memory impact once factorized properly


> b.) the default naming (name="") would break it anyway. You again would
> need to create dynamic qualifiers for each of those injection points.
>

Not true since you handle it in the extension precomputing it






My main concern is the API is rigid and under the CDI constraint instead of
proposing something:

- handled internally by design (fast runtime path)
- enforcing user to modelize the config instead of encouraging to spread it
- including duplication - accross the whole app
- centralizing eviction through the impl instead of having to gather all
usages and evict it in best effort which is not giving any guarantee of
reliability of the code


>
> LieGrue,
> strub
>
> > Am 08.08.2017 um 11:34 schrieb Romain Manni-Bucau  >:
> >
> > we can rollback it, anyway I was not that involved in MP but any hope
> the spec change enough to make the runtime pipeline straight forward and
> avoid any resolution at that time? Don't think the API is strong ATM
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau |  Blog | Old Blog | Github | LinkedIn | JavaEE Factory
> >
> > 2017-08-08 11:33 GMT+02:00 Mark Struberg :
> > Had a talk with Romain via IRC. His goal was to improve the performance
> during injection by providing separate Beans for each Injection Point.
> > We debugged through and in practice the benefit only pays off if you
> have just a single @ConfigProperty Provider for each type T. This will
> imo merely increase the performance for very simple apps and samples. So
> while the goal is good it' imo not worth it as it will not affect real
> world projects. In those you will almost always have multiple
> Provider for example.
> >
> > It was worth a try, but imo the increased complexity (from 50 LOC to 400
> LOC) isn't worth the benefits which only will affect very simple apps
> anyway.
> >
> > Should we go on and ship this version anyway and rework later, or just
> do it and qickly re-roll (will not take long)?
> >
> > LieGrue,
> > strub
> >
> > > Am 08.08.2017 um 09:24 schrieb Mark Struberg :
> > >
> > > Yes that's something else I saw yesterday as well.
> > > Gonna fix that and reroll.
> > >
> > > LieGrue,
> > > strub
> > >
> > >> Am 08.08.2017 um 01:24 schrieb John D. Ament :
> > >>
> > >> Agreed that it would make sense that this required check is skipped
> for provider, optional.  Only Optional is called out though.
> > >>
> > >> At the same time, I think we're over eagerly saying that fields are
> not set.  I raised a spec issue yesterday, basically if the value is
> "org.eclipse.microprofile.config.configproperty.unconfigureddvalue" that
> seems to be what indicates its not set.  However, GConfig is also blowing
> up when it's set to "".
> > >>
> > >> I don't see an issue fixing this on master.  However, right now I
> have master targetting config 1.1.  We can revert back to 1.0 to cut a 1.1
> shortly.
> > >>
> > >> John
> > >>
> > >> On Mon, Aug 7, 2017 at 7:16 PM Mark Struberg 
> wrote:
> > >> It's a requirement for direct injection, but not for Provider afair
> > >>
> > >> Lg,
> > >> Strub
> > >>
> > >> Am 08.08.2017 um 01:03 schrieb John D. Ament :
> > >>
> > >>>
> > >>> I took a look at the test and the commit, to remember why this would
> fail.
> > >>>
> > >>> The test fails because the property isn't set.  This is actually a
> MP Config requirement, validating injection points.  If I add
> System.setProperty(SOME_KEY, "no-op"); to the beginning of the deployment
> method, it works as expected.  See [1] for the API requirements
> > >>>
> > >>> Now that test continues to fail on Weld3 with this change in,
> seemingly the custom provider doesn't work.  Are you using Weld or OWB as
> your runtime?
> > >>>
> > >>> [1]: https://github.com/eclipse/microprofile-config/blob/1.0/
> api/src/main/java/org/eclipse/microprofile/config/inject/
> ConfigProperty.java#L50
> > >>>
> > >>>
> > >>> On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg 
> wrote:
> > >>> I have now tried to use this version in a project and totally blew
> up.
> > >>> I reviewed the code in depth and I'm not sure how to fix it except
> with a revert of some committs.
> > >>>
> > >>> So I gonna change my vote to -1
> > >>>
> > >>> The reason seems to be r1800744 which imo introduced uneccessary
> complexity and broke quite a few features.
> > >>>
> > >>> e.g. injection of Provider is now broken.
> > >>> It also breaks programmatic lookup, etc.
> > >>>
> > >>> How to proceed?
> > >>>
> > >>> LieGrue,
> > >>> strub
> > >>>
> > 

Re: [VOTE] [CANCELLED] Release Apache Geronimo-config 1.0

[Mark Struberg]

Vote is cancelled, we also still miss a binding +1

LieGrue,
strub

> Am 08.08.2017 um 11:40 schrieb Mark Struberg :
> 
> I think the API is quite fine. What are your concrete concerns?
> 
> There are btw 2 reasons why the name in the @ConfigProperty is @Nonbinding:
> 
> a.) you would need to register about 2000 different Beans otherwise (one per 
> injection point basically
> b.) the default naming (name="") would break it anyway. You again would need 
> to create dynamic qualifiers for each of those injection points. 
> 
> LieGrue,
> strub
> 
>> Am 08.08.2017 um 11:34 schrieb Romain Manni-Bucau :
>> 
>> we can rollback it, anyway I was not that involved in MP but any hope the 
>> spec change enough to make the runtime pipeline straight forward and avoid 
>> any resolution at that time? Don't think the API is strong ATM
>> 
>> 
>> Romain Manni-Bucau
>> @rmannibucau |  Blog | Old Blog | Github | LinkedIn | JavaEE Factory
>> 
>> 2017-08-08 11:33 GMT+02:00 Mark Struberg :
>> Had a talk with Romain via IRC. His goal was to improve the performance 
>> during injection by providing separate Beans for each Injection Point.
>> We debugged through and in practice the benefit only pays off if you have 
>> just a single @ConfigProperty Provider for each type T. This will imo 
>> merely increase the performance for very simple apps and samples. So while 
>> the goal is good it' imo not worth it as it will not affect real world 
>> projects. In those you will almost always have multiple Provider for 
>> example.
>> 
>> It was worth a try, but imo the increased complexity (from 50 LOC to 400 
>> LOC) isn't worth the benefits which only will affect very simple apps anyway.
>> 
>> Should we go on and ship this version anyway and rework later, or just do it 
>> and qickly re-roll (will not take long)?
>> 
>> LieGrue,
>> strub
>> 
>>> Am 08.08.2017 um 09:24 schrieb Mark Struberg :
>>> 
>>> Yes that's something else I saw yesterday as well.
>>> Gonna fix that and reroll.
>>> 
>>> LieGrue,
>>> strub
>>> 
 Am 08.08.2017 um 01:24 schrieb John D. Ament :
 
 Agreed that it would make sense that this required check is skipped for 
 provider, optional.  Only Optional is called out though.
 
 At the same time, I think we're over eagerly saying that fields are not 
 set.  I raised a spec issue yesterday, basically if the value is 
 "org.eclipse.microprofile.config.configproperty.unconfigureddvalue" that 
 seems to be what indicates its not set.  However, GConfig is also blowing 
 up when it's set to "".
 
 I don't see an issue fixing this on master.  However, right now I have 
 master targetting config 1.1.  We can revert back to 1.0 to cut a 1.1 
 shortly.
 
 John
 
 On Mon, Aug 7, 2017 at 7:16 PM Mark Struberg  wrote:
 It's a requirement for direct injection, but not for Provider afair
 
 Lg,
 Strub
 
 Am 08.08.2017 um 01:03 schrieb John D. Ament :
 
> 
> I took a look at the test and the commit, to remember why this would fail.
> 
> The test fails because the property isn't set.  This is actually a MP 
> Config requirement, validating injection points.  If I add 
> System.setProperty(SOME_KEY, "no-op"); to the beginning of the deployment 
> method, it works as expected.  See [1] for the API requirements
> 
> Now that test continues to fail on Weld3 with this change in, seemingly 
> the custom provider doesn't work.  Are you using Weld or OWB as your 
> runtime?
> 
> [1]: 
> https://github.com/eclipse/microprofile-config/blob/1.0/api/src/main/java/org/eclipse/microprofile/config/inject/ConfigProperty.java#L50
> 
> 
> On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg  wrote:
> I have now tried to use this version in a project and totally blew up.
> I reviewed the code in depth and I'm not sure how to fix it except with a 
> revert of some committs.
> 
> So I gonna change my vote to -1
> 
> The reason seems to be r1800744 which imo introduced uneccessary 
> complexity and broke quite a few features.
> 
> e.g. injection of Provider is now broken.
> It also breaks programmatic lookup, etc.
> 
> How to proceed?
> 
> LieGrue,
> strub
> 
> 
>> Am 01.08.2017 um 21:54 schrieb Mark Struberg :
>> 
>> And my own +1 of course.
>> 
>> LieGrue,
>> strub
>> 
>> 
>>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner 
>>> :
>>> 
>>> +1 (non-binding)
>>> 
>>> lg
>>> reini
>>> 
 Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré 
 :
 
 +1 (non binding)
 
 Regards
 JB

Re: [VOTE] Release Apache Geronimo-config 1.0

[Mark Struberg]

I think the API is quite fine. What are your concrete concerns?

There are btw 2 reasons why the name in the @ConfigProperty is @Nonbinding:

a.) you would need to register about 2000 different Beans otherwise (one per 
injection point basically
b.) the default naming (name="") would break it anyway. You again would need to 
create dynamic qualifiers for each of those injection points. 

LieGrue,
strub

> Am 08.08.2017 um 11:34 schrieb Romain Manni-Bucau :
> 
> we can rollback it, anyway I was not that involved in MP but any hope the 
> spec change enough to make the runtime pipeline straight forward and avoid 
> any resolution at that time? Don't think the API is strong ATM
> 
> 
> Romain Manni-Bucau
> @rmannibucau |  Blog | Old Blog | Github | LinkedIn | JavaEE Factory
> 
> 2017-08-08 11:33 GMT+02:00 Mark Struberg :
> Had a talk with Romain via IRC. His goal was to improve the performance 
> during injection by providing separate Beans for each Injection Point.
> We debugged through and in practice the benefit only pays off if you have 
> just a single @ConfigProperty Provider for each type T. This will imo 
> merely increase the performance for very simple apps and samples. So while 
> the goal is good it' imo not worth it as it will not affect real world 
> projects. In those you will almost always have multiple Provider for 
> example.
> 
> It was worth a try, but imo the increased complexity (from 50 LOC to 400 LOC) 
> isn't worth the benefits which only will affect very simple apps anyway.
> 
> Should we go on and ship this version anyway and rework later, or just do it 
> and qickly re-roll (will not take long)?
> 
> LieGrue,
> strub
> 
> > Am 08.08.2017 um 09:24 schrieb Mark Struberg :
> >
> > Yes that's something else I saw yesterday as well.
> > Gonna fix that and reroll.
> >
> > LieGrue,
> > strub
> >
> >> Am 08.08.2017 um 01:24 schrieb John D. Ament :
> >>
> >> Agreed that it would make sense that this required check is skipped for 
> >> provider, optional.  Only Optional is called out though.
> >>
> >> At the same time, I think we're over eagerly saying that fields are not 
> >> set.  I raised a spec issue yesterday, basically if the value is 
> >> "org.eclipse.microprofile.config.configproperty.unconfigureddvalue" that 
> >> seems to be what indicates its not set.  However, GConfig is also blowing 
> >> up when it's set to "".
> >>
> >> I don't see an issue fixing this on master.  However, right now I have 
> >> master targetting config 1.1.  We can revert back to 1.0 to cut a 1.1 
> >> shortly.
> >>
> >> John
> >>
> >> On Mon, Aug 7, 2017 at 7:16 PM Mark Struberg  wrote:
> >> It's a requirement for direct injection, but not for Provider afair
> >>
> >> Lg,
> >> Strub
> >>
> >> Am 08.08.2017 um 01:03 schrieb John D. Ament :
> >>
> >>>
> >>> I took a look at the test and the commit, to remember why this would fail.
> >>>
> >>> The test fails because the property isn't set.  This is actually a MP 
> >>> Config requirement, validating injection points.  If I add 
> >>> System.setProperty(SOME_KEY, "no-op"); to the beginning of the deployment 
> >>> method, it works as expected.  See [1] for the API requirements
> >>>
> >>> Now that test continues to fail on Weld3 with this change in, seemingly 
> >>> the custom provider doesn't work.  Are you using Weld or OWB as your 
> >>> runtime?
> >>>
> >>> [1]: 
> >>> https://github.com/eclipse/microprofile-config/blob/1.0/api/src/main/java/org/eclipse/microprofile/config/inject/ConfigProperty.java#L50
> >>>
> >>>
> >>> On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg  wrote:
> >>> I have now tried to use this version in a project and totally blew up.
> >>> I reviewed the code in depth and I'm not sure how to fix it except with a 
> >>> revert of some committs.
> >>>
> >>> So I gonna change my vote to -1
> >>>
> >>> The reason seems to be r1800744 which imo introduced uneccessary 
> >>> complexity and broke quite a few features.
> >>>
> >>> e.g. injection of Provider is now broken.
> >>> It also breaks programmatic lookup, etc.
> >>>
> >>> How to proceed?
> >>>
> >>> LieGrue,
> >>> strub
> >>>
> >>>
>  Am 01.08.2017 um 21:54 schrieb Mark Struberg :
> 
>  And my own +1 of course.
> 
>  LieGrue,
>  strub
> 
> 
> > Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner 
> > :
> >
> > +1 (non-binding)
> >
> > lg
> > reini
> >
> >> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré 
> >> :
> >>
> >> +1 (non binding)
> >>
> >> Regards
> >> JB
> >> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO  
> >> wrote:
> >> +1
> >> Thanks
> >>
> >> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau 
> >>  a écrit 

Re: [VOTE] Release Apache Geronimo-config 1.0

[Romain Manni-Bucau]

we can rollback it, anyway I was not that involved in MP but any hope the
spec change enough to make the runtime pipeline straight forward and avoid
any resolution at that time? Don't think the API is strong ATM


Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | JavaEE Factory


2017-08-08 11:33 GMT+02:00 Mark Struberg :

> Had a talk with Romain via IRC. His goal was to improve the performance
> during injection by providing separate Beans for each Injection Point.
> We debugged through and in practice the benefit only pays off if you have
> just a single @ConfigProperty Provider for each type T. This will imo
> merely increase the performance for very simple apps and samples. So while
> the goal is good it' imo not worth it as it will not affect real world
> projects. In those you will almost always have multiple Provider
> for example.
>
> It was worth a try, but imo the increased complexity (from 50 LOC to 400
> LOC) isn't worth the benefits which only will affect very simple apps
> anyway.
>
> Should we go on and ship this version anyway and rework later, or just do
> it and qickly re-roll (will not take long)?
>
> LieGrue,
> strub
>
> > Am 08.08.2017 um 09:24 schrieb Mark Struberg :
> >
> > Yes that's something else I saw yesterday as well.
> > Gonna fix that and reroll.
> >
> > LieGrue,
> > strub
> >
> >> Am 08.08.2017 um 01:24 schrieb John D. Ament :
> >>
> >> Agreed that it would make sense that this required check is skipped for
> provider, optional.  Only Optional is called out though.
> >>
> >> At the same time, I think we're over eagerly saying that fields are not
> set.  I raised a spec issue yesterday, basically if the value is
> "org.eclipse.microprofile.config.configproperty.unconfigureddvalue" that
> seems to be what indicates its not set.  However, GConfig is also blowing
> up when it's set to "".
> >>
> >> I don't see an issue fixing this on master.  However, right now I have
> master targetting config 1.1.  We can revert back to 1.0 to cut a 1.1
> shortly.
> >>
> >> John
> >>
> >> On Mon, Aug 7, 2017 at 7:16 PM Mark Struberg  wrote:
> >> It's a requirement for direct injection, but not for Provider afair
> >>
> >> Lg,
> >> Strub
> >>
> >> Am 08.08.2017 um 01:03 schrieb John D. Ament :
> >>
> >>>
> >>> I took a look at the test and the commit, to remember why this would
> fail.
> >>>
> >>> The test fails because the property isn't set.  This is actually a MP
> Config requirement, validating injection points.  If I add
> System.setProperty(SOME_KEY, "no-op"); to the beginning of the deployment
> method, it works as expected.  See [1] for the API requirements
> >>>
> >>> Now that test continues to fail on Weld3 with this change in,
> seemingly the custom provider doesn't work.  Are you using Weld or OWB as
> your runtime?
> >>>
> >>> [1]: https://github.com/eclipse/microprofile-config/blob/1.0/
> api/src/main/java/org/eclipse/microprofile/config/inject/
> ConfigProperty.java#L50
> >>>
> >>>
> >>> On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg 
> wrote:
> >>> I have now tried to use this version in a project and totally blew up.
> >>> I reviewed the code in depth and I'm not sure how to fix it except
> with a revert of some committs.
> >>>
> >>> So I gonna change my vote to -1
> >>>
> >>> The reason seems to be r1800744 which imo introduced uneccessary
> complexity and broke quite a few features.
> >>>
> >>> e.g. injection of Provider is now broken.
> >>> It also breaks programmatic lookup, etc.
> >>>
> >>> How to proceed?
> >>>
> >>> LieGrue,
> >>> strub
> >>>
> >>>
>  Am 01.08.2017 um 21:54 schrieb Mark Struberg :
> 
>  And my own +1 of course.
> 
>  LieGrue,
>  strub
> 
> 
> > Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <
> reinhard.sandt...@gmail.com>:
> >
> > +1 (non-binding)
> >
> > lg
> > reini
> >
> >> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <
> j...@nanthrax.net>:
> >>
> >> +1 (non binding)
> >>
> >> Regards
> >> JB
> >> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO 
> wrote:
> >> +1
> >> Thanks
> >>
> >> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <
> rmannibu...@gmail.com> a écrit :
> >> +1 (for the release and the src zip in dist)
> >>
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau |  Blog | Old Blog | Github | LinkedIn | JavaEE Factory
> >>
> >> 2017-07-28 14:17 GMT+02:00 Mark Struberg :
> >> Yes we should make sure this gets propagated to dist!
> >>
> >> LieGrue,
> >> strub
> >>

Re: [VOTE] Release Apache Geronimo-config 1.0

[Mark Struberg]

Had a talk with Romain via IRC. His goal was to improve the performance during 
injection by providing separate Beans for each Injection Point. 
We debugged through and in practice the benefit only pays off if you have just 
a single @ConfigProperty Provider for each type T. This will imo merely 
increase the performance for very simple apps and samples. So while the goal is 
good it' imo not worth it as it will not affect real world projects. In those 
you will almost always have multiple Provider for example.

It was worth a try, but imo the increased complexity (from 50 LOC to 400 LOC) 
isn't worth the benefits which only will affect very simple apps anyway.

Should we go on and ship this version anyway and rework later, or just do it 
and qickly re-roll (will not take long)?

LieGrue,
strub

> Am 08.08.2017 um 09:24 schrieb Mark Struberg :
> 
> Yes that's something else I saw yesterday as well. 
> Gonna fix that and reroll.
> 
> LieGrue,
> strub
> 
>> Am 08.08.2017 um 01:24 schrieb John D. Ament :
>> 
>> Agreed that it would make sense that this required check is skipped for 
>> provider, optional.  Only Optional is called out though.
>> 
>> At the same time, I think we're over eagerly saying that fields are not set. 
>>  I raised a spec issue yesterday, basically if the value is 
>> "org.eclipse.microprofile.config.configproperty.unconfigureddvalue" that 
>> seems to be what indicates its not set.  However, GConfig is also blowing up 
>> when it's set to "".
>> 
>> I don't see an issue fixing this on master.  However, right now I have 
>> master targetting config 1.1.  We can revert back to 1.0 to cut a 1.1 
>> shortly.
>> 
>> John
>> 
>> On Mon, Aug 7, 2017 at 7:16 PM Mark Struberg  wrote:
>> It's a requirement for direct injection, but not for Provider afair
>> 
>> Lg,
>> Strub
>> 
>> Am 08.08.2017 um 01:03 schrieb John D. Ament :
>> 
>>> 
>>> I took a look at the test and the commit, to remember why this would fail.
>>> 
>>> The test fails because the property isn't set.  This is actually a MP 
>>> Config requirement, validating injection points.  If I add 
>>> System.setProperty(SOME_KEY, "no-op"); to the beginning of the deployment 
>>> method, it works as expected.  See [1] for the API requirements
>>> 
>>> Now that test continues to fail on Weld3 with this change in, seemingly the 
>>> custom provider doesn't work.  Are you using Weld or OWB as your runtime?
>>> 
>>> [1]: 
>>> https://github.com/eclipse/microprofile-config/blob/1.0/api/src/main/java/org/eclipse/microprofile/config/inject/ConfigProperty.java#L50
>>> 
>>> 
>>> On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg  wrote:
>>> I have now tried to use this version in a project and totally blew up.
>>> I reviewed the code in depth and I'm not sure how to fix it except with a 
>>> revert of some committs.
>>> 
>>> So I gonna change my vote to -1
>>> 
>>> The reason seems to be r1800744 which imo introduced uneccessary complexity 
>>> and broke quite a few features.
>>> 
>>> e.g. injection of Provider is now broken.
>>> It also breaks programmatic lookup, etc.
>>> 
>>> How to proceed?
>>> 
>>> LieGrue,
>>> strub
>>> 
>>> 
 Am 01.08.2017 um 21:54 schrieb Mark Struberg :
 
 And my own +1 of course.
 
 LieGrue,
 strub
 
 
> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner 
> :
> 
> +1 (non-binding)
> 
> lg
> reini
> 
>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré :
>> 
>> +1 (non binding)
>> 
>> Regards
>> JB
>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO  
>> wrote:
>> +1
>> Thanks
>> 
>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau 
>>  a écrit :
>> +1 (for the release and the src zip in dist)
>> 
>> 
>> Romain Manni-Bucau
>> @rmannibucau |  Blog | Old Blog | Github | LinkedIn | JavaEE Factory
>> 
>> 2017-07-28 14:17 GMT+02:00 Mark Struberg :
>> Yes we should make sure this gets propagated to dist!
>> 
>> LieGrue,
>> strub
>> 
>>> Am 28.07.2017 um 13:58 schrieb John D. Ament :
>>> 
>>> +1 to ship it.
>>> 
>>> One comment, there's been input in the past that geronimo releases 
>>> don't show up in the system.  Should we ensure that the final result 
>>> gets put into https://dist.apache.org/repos/dist/release/geronimo/ ?
>>> 
>>> On Fri, Jul 28, 2017 at 7:25 AM Mark Struberg  wrote:
>>> Hi!
>>> 
>>> Apache geronimo-config is an implementation of the Microprofile-1.0 
>>> Config specification [1][2].
>>> 
>>> It allows flexible and extensible Configuration for applications.
>>> 
>>> 
>>> Here is our staging repo
>>> 

Re: [VOTE] Release Apache Geronimo-config 1.0

[Mark Struberg]

Yes that's something else I saw yesterday as well. 
Gonna fix that and reroll.

LieGrue,
strub

> Am 08.08.2017 um 01:24 schrieb John D. Ament :
> 
> Agreed that it would make sense that this required check is skipped for 
> provider, optional.  Only Optional is called out though.
> 
> At the same time, I think we're over eagerly saying that fields are not set.  
> I raised a spec issue yesterday, basically if the value is 
> "org.eclipse.microprofile.config.configproperty.unconfigureddvalue" that 
> seems to be what indicates its not set.  However, GConfig is also blowing up 
> when it's set to "".
> 
> I don't see an issue fixing this on master.  However, right now I have master 
> targetting config 1.1.  We can revert back to 1.0 to cut a 1.1 shortly.
> 
> John
> 
> On Mon, Aug 7, 2017 at 7:16 PM Mark Struberg  wrote:
> It's a requirement for direct injection, but not for Provider afair
> 
> Lg,
> Strub
> 
> Am 08.08.2017 um 01:03 schrieb John D. Ament :
> 
>> 
>> I took a look at the test and the commit, to remember why this would fail.
>> 
>> The test fails because the property isn't set.  This is actually a MP Config 
>> requirement, validating injection points.  If I add 
>> System.setProperty(SOME_KEY, "no-op"); to the beginning of the deployment 
>> method, it works as expected.  See [1] for the API requirements
>> 
>> Now that test continues to fail on Weld3 with this change in, seemingly the 
>> custom provider doesn't work.  Are you using Weld or OWB as your runtime?
>> 
>> [1]: 
>> https://github.com/eclipse/microprofile-config/blob/1.0/api/src/main/java/org/eclipse/microprofile/config/inject/ConfigProperty.java#L50
>> 
>> 
>> On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg  wrote:
>> I have now tried to use this version in a project and totally blew up.
>> I reviewed the code in depth and I'm not sure how to fix it except with a 
>> revert of some committs.
>> 
>> So I gonna change my vote to -1
>> 
>> The reason seems to be r1800744 which imo introduced uneccessary complexity 
>> and broke quite a few features.
>> 
>> e.g. injection of Provider is now broken.
>> It also breaks programmatic lookup, etc.
>> 
>> How to proceed?
>> 
>> LieGrue,
>> strub
>> 
>> 
>> > Am 01.08.2017 um 21:54 schrieb Mark Struberg :
>> >
>> > And my own +1 of course.
>> >
>> > LieGrue,
>> > strub
>> >
>> >
>> >> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner 
>> >> :
>> >>
>> >> +1 (non-binding)
>> >>
>> >> lg
>> >> reini
>> >>
>> >>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré :
>> >>>
>> >>> +1 (non binding)
>> >>>
>> >>> Regards
>> >>> JB
>> >>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO  
>> >>> wrote:
>> >>> +1
>> >>> Thanks
>> >>>
>> >>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau 
>> >>>  a écrit :
>> >>> +1 (for the release and the src zip in dist)
>> >>>
>> >>>
>> >>> Romain Manni-Bucau
>> >>> @rmannibucau |  Blog | Old Blog | Github | LinkedIn | JavaEE Factory
>> >>>
>> >>> 2017-07-28 14:17 GMT+02:00 Mark Struberg :
>> >>> Yes we should make sure this gets propagated to dist!
>> >>>
>> >>> LieGrue,
>> >>> strub
>> >>>
>>  Am 28.07.2017 um 13:58 schrieb John D. Ament :
>> 
>>  +1 to ship it.
>> 
>>  One comment, there's been input in the past that geronimo releases 
>>  don't show up in the system.  Should we ensure that the final result 
>>  gets put into https://dist.apache.org/repos/dist/release/geronimo/ ?
>> 
>>  On Fri, Jul 28, 2017 at 7:25 AM Mark Struberg  wrote:
>>  Hi!
>> 
>>  Apache geronimo-config is an implementation of the Microprofile-1.0 
>>  Config specification [1][2].
>> 
>>  It allows flexible and extensible Configuration for applications.
>> 
>> 
>>  Here is our staging repo
>>  https://repository.apache.org/content/repositories/orgapachegeronimo-1035/
>> 
>>  The Source distribution can be found here:
>>  https://repository.apache.org/content/repositories/orgapachegeronimo-1035/org/apache/geronimo/config/config-parent/1.0/
>> 
>> 
>>  Our own tag in SVN is
>>  https://svn.apache.org/repos/asf/geronimo/components/config/tags/config-parent-1.0/
>> 
>> 
>>  Please VOTE
>>  [+1] yeah, ship it!
>>  [+0] meh, don't care
>>  [-1] nope, because ${showstopper}
>> 
>>  The VOTE is open for 72h
>> 
>>  txs and LieGrue,
>>  strub
>> 
>> 
>> 
>>  [1] https://github.com/eclipse/microprofile-config
>>  [2] https://github.com/eclipse/microprofile-config/releases
>> 
>> >>>
>> >>>
>> >>
>> >
>>