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 <rmannibu...@gmail.com>: > > 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 <strub...@yahoo.de>: > 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 <strub...@yahoo.de>: > > > > 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 <johndam...@apache.org>: > >> > >> 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 <strub...@yahoo.de> 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 <johndam...@apache.org>: > >> > >>> > >>> 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 <strub...@yahoo.de> 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 <strub...@yahoo.de>: > >>>> > >>>> 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 <jeano...@gmail.com> > >>>>>> 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 <strub...@yahoo.de>: > >>>>>> Yes we should make sure this gets propagated to dist! > >>>>>> > >>>>>> LieGrue, > >>>>>> strub > >>>>>> > >>>>>>> Am 28.07.2017 um 13:58 schrieb John D. Ament <johndam...@apache.org>: > >>>>>>> > >>>>>>> +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 <strub...@yahoo.de> > >>>>>>> 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 > >>>>>>> > >>>>>> > >>>>>> > >>>>> > >>>> > >>> > > > >
