Think we need to: 1. make impl shorter providing few default impl base - mainly ones from impl module ensuring there are public and "@Stable" 2. Support more cdi captures - not only PropertyFileSource even if limited as you pointed out (works for app which config the extensions in embedded files ~= internal app config vs external envrt values)
Wdyt? Le 21 août 2016 21:48, "Mark Struberg" <[email protected]> a écrit : > > Thanks for adding a link to the config stuff in a more visible place! > > Yes, I also think we should have those classes public (but in the impl module). > > Regarding the PropertyFileConfig: > I think I documented this, but better mention it again: > You can register it in 2 ways > > 1.) Simply implement the interface > -> in that case it gets picked up via ProcessAnnotatedType and will _only_ be available after the container did finish booting! > That means that you cannot use values from those ConfigSources during CDI boot, e.g. for @Exclude > > 2.) To work around the timing issue in 1 we later also made it pick up via java.util.ServiceLoader. In that case it will get picked up like a regular ConfigSource and is also available during container bootstrap. > > Just don't fortet to add a @Exclude annotation to prevent the PropertyFileConfig from being picked up twice. > > > Happy to get any feedback about how to improve the docs if something is missing. > > > LieGrue, > strub > > > > > > On Sunday, 21 August 2016, 20:57, John D. Ament <[email protected]> wrote: > > > On Sun, Aug 21, 2016 at 2:44 PM Mark Struberg <[email protected]> > > wrote: > > > >> Oh if it's not public then we should really make it! > >> > >> You are right with 'production code'. Of course it's in > > production. > >> But it's more like infrastructure glue code and not business related, > >> right? > >> Means your colleagues tinkering on UI and business should typically not > >> even get it as compile scoped dependency but really only as runtime dep. > >> John, romain, do you agree on this point? > >> > > > > Right, the problem is that the class isn't even public. So even if I > > declare a proper dependency on it, can't be used. I would say my view > > point is that its SPI, along with a few other classes (MapConfigSource, > > PropertiesConfigSource, PropertyFileConfigSource). Realistically though, > > the problem I was trying to solve was resolved by implementing > > "PropertyFileConfig" but trying to remember how that should work, I > > couldn't even find the doc page (eventually found it, its mentioned briefly > > on core, but really should be its own link on the landing page, which I > > changed). > > > > I could also see the class not being SPI. As long as things are configured > > properly, its not needed. That's assuming we have a dynamic way to > > register more property files. Right now, that's not possible > > (PropertyFileConfig is only read once, and only via ServiceLoader). > > > > Maybe thats the better discussion to have. > > > > > > > >> Just trying to make a few steps back to look at the problem from a > >> distance... > >> > >> LieGrue, > >> Strub > >> > >> > Am 21.08.2016 um 16:11 schrieb Romain Manni-Bucau > > <[email protected] > >> >: > >> > > >> > 2016-08-21 16:08 GMT+02:00 Mark Struberg > > <[email protected]>: > >> > > >> >> EnvironmentPropertyConfigSourceProvider is effectively an internal > >> class. > >> >> And thus I'd not move it to the API. Just look how much we > > would need to > >> >> add to the api package. > >> >> > >> >> I understand that you don't like to duplicate code. > >> >> > >> >> Usually things like ConfigSources are totally orthogonal to your > >> >> production code. So you need those only as runtime dependency. > >> > I get the idea but this is likely not what you wanted to say. > > Typically > >> > sources are production code (but not business code). > >> > > >> > > >> >> > >> >> In that case just make a tool module which has ds-core-impl as > > compile > >> >> time dependency and then you can simply re-use and even extend the > >> internal > >> >> class. > >> >> But that class is clearly NOT an API class imo. > >> > Question is: how stable do we consider -impl is? If the solution to > > that > >> > issue is 1. make the class (whichever it is) public 2. say to the user > > to > >> > import -impl in his pom, then we need to show that the API is stable > > and > >> > not just an internal we can break when we want. @Stable would be an > >> option > >> > but moving it to -api is another one. Don't care of the solution > > while we > >> > prevent the user to duplicate our -impl code. > >> > > >> > > >> >> LieGrue, > >> >> strub > >> >> > >> >> > >> >> > >> >> > >> >> > >> >>> On Sunday, 21 August 2016, 15:28, John D. Ament > > <[email protected] > >> > > >> >> wrote: > >> >>>> Hi, > >> >>> > >> >>> I have a use case for leveraging something > >> >>> like EnvironmentPropertyConfigSourceProvider to load multiple > > property > >> >>> files, outside of apache-deltaspike.properties. Right now its > > package > >> >>> local and in the impl JAR. I'd rather not duplicate the > > logic since > >> >> whats > >> >>> being done is exactly what is needed. I was wondering if > > anyone had > >> any > >> >>> concerns with making this an API class instead of an impl > > class? > >> >>> > >> >>> John > >> >> > >> > >> > >
