[VOTE] Release Apache Geronimo-config 1.0

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[VOTE] Release Apache Geronimo-config 1.0

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

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 <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

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 <[hidden email]>:
>
> +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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

Romain Manni-Bucau
+1 (for the release and the src zip in dist)


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

2017-07-28 14:17 GMT+02:00 Mark Struberg <[hidden email]>:
Yes we should make sure this gets propagated to dist!

LieGrue,
strub

> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>
> +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 <[hidden email]> 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
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

jlmonteiro
+1 
Thanks

Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> a écrit :
+1 (for the release and the src zip in dist)


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

2017-07-28 14:17 GMT+02:00 Mark Struberg <[hidden email]>:
Yes we should make sure this gets propagated to dist!

LieGrue,
strub

> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>
> +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 <[hidden email]> 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
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

Jean-Baptiste Onofré
+1 (non binding)

Regards
JB
On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
+1 
Thanks

Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> a écrit :
+1 (for the release and the src zip in dist)


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

2017-07-28 14:17 GMT+02:00 Mark Struberg <[hidden email]>:
Yes we should make sure this gets propagated to dist!

LieGrue,
strub

> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>
> +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 <[hidden email]> 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
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

Reinhard Sandtner-2
+1 (non-binding)

lg
reini

Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:

+1 (non binding)

Regards
JB
On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
+1 
Thanks

Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> a écrit :
+1 (for the release and the src zip in dist)


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

2017-07-28 14:17 GMT+02:00 Mark Struberg <[hidden email]>:
Yes we should make sure this gets propagated to dist!

LieGrue,
strub

> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>
> +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 <[hidden email]> 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
>



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

Mark Struberg
And my own +1 of course.

LieGrue,
strub


> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>
> +1 (non-binding)
>
> lg
> reini
>
>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>>
>> +1 (non binding)
>>
>> Regards
>> JB
>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>> +1
>> Thanks
>>
>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>> Yes we should make sure this gets propagated to dist!
>>
>> LieGrue,
>> strub
>>
>> > Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>> >
>> > +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 <[hidden email]> 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
>> >
>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

Mark Struberg
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 <[hidden email]>:
>
> And my own +1 of course.
>
> LieGrue,
> strub
>
>
>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>>
>> +1 (non-binding)
>>
>> lg
>> reini
>>
>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>>>
>>> +1 (non binding)
>>>
>>> Regards
>>> JB
>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>>> +1
>>> Thanks
>>>
>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>>> Yes we should make sure this gets propagated to dist!
>>>
>>> LieGrue,
>>> strub
>>>
>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>>>>
>>>> +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 <[hidden email]> 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
>>>>
>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

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?



On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg <[hidden email]> 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 <[hidden email]>:
>
> And my own +1 of course.
>
> LieGrue,
> strub
>
>
>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>>
>> +1 (non-binding)
>>
>> lg
>> reini
>>
>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>>>
>>> +1 (non binding)
>>>
>>> Regards
>>> JB
>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>>> +1
>>> Thanks
>>>
>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>>> Yes we should make sure this gets propagated to dist!
>>>
>>> LieGrue,
>>> strub
>>>
>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>>>>
>>>> +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 <[hidden email]> 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
>>>>
>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

Mark Struberg
It's a requirement for direct injection, but not for Provider<T> afair

Lg,
Strub

Am 08.08.2017 um 01:03 schrieb John D. Ament <[hidden email]>:


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?



On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg <[hidden email]> 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 <[hidden email]>:
>
> And my own +1 of course.
>
> LieGrue,
> strub
>
>
>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>>
>> +1 (non-binding)
>>
>> lg
>> reini
>>
>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>>>
>>> +1 (non binding)
>>>
>>> Regards
>>> JB
>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>>> +1
>>> Thanks
>>>
>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>>> Yes we should make sure this gets propagated to dist!
>>>
>>> LieGrue,
>>> strub
>>>
>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>>>>
>>>> +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 <[hidden email]> 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
>>>>
>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

Mark Struberg
Did re read the section. You are right. it's mandatory for Provider and only dont need to exist for Optional.

The message is def wrong though, but that might not be a showstopper.
Will sleep over it.

Lg,
Strub

Am 08.08.2017 um 01:16 schrieb Mark Struberg <[hidden email]>:

It's a requirement for direct injection, but not for Provider<T> afair

Lg,
Strub

Am 08.08.2017 um 01:03 schrieb John D. Ament <[hidden email]>:


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?



On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg <[hidden email]> 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 <[hidden email]>:
>
> And my own +1 of course.
>
> LieGrue,
> strub
>
>
>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>>
>> +1 (non-binding)
>>
>> lg
>> reini
>>
>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>>>
>>> +1 (non binding)
>>>
>>> Regards
>>> JB
>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>>> +1
>>> Thanks
>>>
>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>>> Yes we should make sure this gets propagated to dist!
>>>
>>> LieGrue,
>>> strub
>>>
>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>>>>
>>>> +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 <[hidden email]> 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
>>>>
>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

John D. Ament
In reply to this post by Mark Struberg
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 <[hidden email]> wrote:
It's a requirement for direct injection, but not for Provider<T> afair

Lg,
Strub

Am 08.08.2017 um 01:03 schrieb John D. Ament <[hidden email]>:


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?



On Mon, Aug 7, 2017 at 5:27 PM Mark Struberg <[hidden email]> 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 <[hidden email]>:
>
> And my own +1 of course.
>
> LieGrue,
> strub
>
>
>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>>
>> +1 (non-binding)
>>
>> lg
>> reini
>>
>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>>>
>>> +1 (non binding)
>>>
>>> Regards
>>> JB
>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>>> +1
>>> Thanks
>>>
>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>>> Yes we should make sure this gets propagated to dist!
>>>
>>> LieGrue,
>>> strub
>>>
>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>>>>
>>>> +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 <[hidden email]> 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
>>>>
>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 <[hidden email]>:
>
> 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 <[hidden email]> wrote:
> It's a requirement for direct injection, but not for Provider<T> afair
>
> Lg,
> Strub
>
> Am 08.08.2017 um 01:03 schrieb John D. Ament <[hidden email]>:
>
>>
>> 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 <[hidden email]> 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 <[hidden email]>:
>> >
>> > And my own +1 of course.
>> >
>> > LieGrue,
>> > strub
>> >
>> >
>> >> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>> >>
>> >> +1 (non-binding)
>> >>
>> >> lg
>> >> reini
>> >>
>> >>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>> >>>
>> >>> +1 (non binding)
>> >>>
>> >>> Regards
>> >>> JB
>> >>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>> >>> +1
>> >>> Thanks
>> >>>
>> >>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>> >>> Yes we should make sure this gets propagated to dist!
>> >>>
>> >>> LieGrue,
>> >>> strub
>> >>>
>> >>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>> >>>>
>> >>>> +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 <[hidden email]> 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
>> >>>>
>> >>>
>> >>>
>> >>
>> >
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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<T> 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<String> 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 <[hidden email]>:
>
> 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 <[hidden email]>:
>>
>> 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 <[hidden email]> wrote:
>> It's a requirement for direct injection, but not for Provider<T> afair
>>
>> Lg,
>> Strub
>>
>> Am 08.08.2017 um 01:03 schrieb John D. Ament <[hidden email]>:
>>
>>>
>>> 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 <[hidden email]> 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 <[hidden email]>:
>>>>
>>>> And my own +1 of course.
>>>>
>>>> LieGrue,
>>>> strub
>>>>
>>>>
>>>>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>>>>>
>>>>> +1 (non-binding)
>>>>>
>>>>> lg
>>>>> reini
>>>>>
>>>>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>>>>>>
>>>>>> +1 (non binding)
>>>>>>
>>>>>> Regards
>>>>>> JB
>>>>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>>>>>> +1
>>>>>> Thanks
>>>>>>
>>>>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>>>>>> Yes we should make sure this gets propagated to dist!
>>>>>>
>>>>>> LieGrue,
>>>>>> strub
>>>>>>
>>>>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>>>>>>>
>>>>>>> +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 <[hidden email]> 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
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 BlogGithub | LinkedIn | JavaEE Factory

2017-08-08 11:33 GMT+02:00 Mark Struberg <[hidden email]>:
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<T> 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<String> 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 <[hidden email]>:
>
> 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 <[hidden email]>:
>>
>> 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 <[hidden email]> wrote:
>> It's a requirement for direct injection, but not for Provider<T> afair
>>
>> Lg,
>> Strub
>>
>> Am 08.08.2017 um 01:03 schrieb John D. Ament <[hidden email]>:
>>
>>>
>>> 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 <[hidden email]> 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 <[hidden email]>:
>>>>
>>>> And my own +1 of course.
>>>>
>>>> LieGrue,
>>>> strub
>>>>
>>>>
>>>>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>>>>>
>>>>> +1 (non-binding)
>>>>>
>>>>> lg
>>>>> reini
>>>>>
>>>>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>>>>>>
>>>>>> +1 (non binding)
>>>>>>
>>>>>> Regards
>>>>>> JB
>>>>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>>>>>> +1
>>>>>> Thanks
>>>>>>
>>>>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>>>>>> Yes we should make sure this gets propagated to dist!
>>>>>>
>>>>>> LieGrue,
>>>>>> strub
>>>>>>
>>>>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>>>>>>>
>>>>>>> +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 <[hidden email]> 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
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 <[hidden email]>:
>
> 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 <[hidden email]>:
> 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<T> 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<String> 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 <[hidden email]>:
> >
> > 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 <[hidden email]>:
> >>
> >> 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 <[hidden email]> wrote:
> >> It's a requirement for direct injection, but not for Provider<T> afair
> >>
> >> Lg,
> >> Strub
> >>
> >> Am 08.08.2017 um 01:03 schrieb John D. Ament <[hidden email]>:
> >>
> >>>
> >>> 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 <[hidden email]> 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 <[hidden email]>:
> >>>>
> >>>> And my own +1 of course.
> >>>>
> >>>> LieGrue,
> >>>> strub
> >>>>
> >>>>
> >>>>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
> >>>>>
> >>>>> +1 (non-binding)
> >>>>>
> >>>>> lg
> >>>>> reini
> >>>>>
> >>>>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
> >>>>>>
> >>>>>> +1 (non binding)
> >>>>>>
> >>>>>> Regards
> >>>>>> JB
> >>>>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
> >>>>>> +1
> >>>>>> Thanks
> >>>>>>
> >>>>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
> >>>>>> Yes we should make sure this gets propagated to dist!
> >>>>>>
> >>>>>> LieGrue,
> >>>>>> strub
> >>>>>>
> >>>>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
> >>>>>>>
> >>>>>>> +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 <[hidden email]> 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
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >
>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 <[hidden email]>:
>
> 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 <[hidden email]>:
>>
>> 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 <[hidden email]>:
>> 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<T> 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<String> 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 <[hidden email]>:
>>>
>>> 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 <[hidden email]>:
>>>>
>>>> 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 <[hidden email]> wrote:
>>>> It's a requirement for direct injection, but not for Provider<T> afair
>>>>
>>>> Lg,
>>>> Strub
>>>>
>>>> Am 08.08.2017 um 01:03 schrieb John D. Ament <[hidden email]>:
>>>>
>>>>>
>>>>> 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 <[hidden email]> 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 <[hidden email]>:
>>>>>>
>>>>>> And my own +1 of course.
>>>>>>
>>>>>> LieGrue,
>>>>>> strub
>>>>>>
>>>>>>
>>>>>>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
>>>>>>>
>>>>>>> +1 (non-binding)
>>>>>>>
>>>>>>> lg
>>>>>>> reini
>>>>>>>
>>>>>>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
>>>>>>>>
>>>>>>>> +1 (non binding)
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> JB
>>>>>>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
>>>>>>>> +1
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
>>>>>>>> Yes we should make sure this gets propagated to dist!
>>>>>>>>
>>>>>>>> LieGrue,
>>>>>>>> strub
>>>>>>>>
>>>>>>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
>>>>>>>>>
>>>>>>>>> +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 <[hidden email]> 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
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [VOTE] Release Apache Geronimo-config 1.0

Romain Manni-Bucau
In reply to this post by Mark Struberg


2017-08-08 11:40 GMT+02:00 Mark Struberg <[hidden email]>:
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 <[hidden email]>:
>
> 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 <[hidden email]>:
> 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<T> 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<String> 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 <[hidden email]>:
> >
> > 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 <[hidden email]>:
> >>
> >> 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 <[hidden email]> wrote:
> >> It's a requirement for direct injection, but not for Provider<T> afair
> >>
> >> Lg,
> >> Strub
> >>
> >> Am 08.08.2017 um 01:03 schrieb John D. Ament <[hidden email]>:
> >>
> >>>
> >>> 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 <[hidden email]> 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 <[hidden email]>:
> >>>>
> >>>> And my own +1 of course.
> >>>>
> >>>> LieGrue,
> >>>> strub
> >>>>
> >>>>
> >>>>> Am 01.08.2017 um 11:16 schrieb Reinhard Sandtner <[hidden email]>:
> >>>>>
> >>>>> +1 (non-binding)
> >>>>>
> >>>>> lg
> >>>>> reini
> >>>>>
> >>>>>> Am 30.07.2017 um 12:16 schrieb Jean-Baptiste Onofré <[hidden email]>:
> >>>>>>
> >>>>>> +1 (non binding)
> >>>>>>
> >>>>>> Regards
> >>>>>> JB
> >>>>>> On Jul 30, 2017, at 12:04, Jean-Louis MONTEIRO <[hidden email]> wrote:
> >>>>>> +1
> >>>>>> Thanks
> >>>>>>
> >>>>>> Le ven. 28 juil. 2017 à 13:20, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]>:
> >>>>>> Yes we should make sure this gets propagated to dist!
> >>>>>>
> >>>>>> LieGrue,
> >>>>>> strub
> >>>>>>
> >>>>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <[hidden email]>:
> >>>>>>>
> >>>>>>> +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 <[hidden email]> 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
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >
>
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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<String>) 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 <[hidden email]>:
>
> 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
>

12
Loading...