Apologies, I had a diff open and misread the change. My points about defaulting the behaviour and configurable key prefix/suffix remains. If it wasn't clear, my comment is that this should be the default behaviour of the other methods. Maybe not now for backwards compatibility, but at some point.
John On Fri, Apr 15, 2016 at 7:11 AM Mark Struberg <strub...@yahoo.de> wrote: > It’s not duplicated. The variable logic is implemented exactly once right > now. > > LieGrue, > strub > > > > Am 15.04.2016 um 12:52 schrieb John D. Ament <johndam...@apache.org>: > > > > Hi Mark, > > > > Thanks for bringing up this discussion! > > > > The way its implemented looks fine. When I first read your email, it > sounded like you were putting the onus on the config source to do the > variable expansion. This way allows you to cleanly have different sources > mixing variables together. > > > > Looking at the way its implemented, I would say that the non-expanding > methods should just overload and delegate into the expanding version. > Right now it looks like that logic is duplicated. What you're doing makes > sense for backwards compatibility but longer term I would imagine we want > filtering on by default. > > > > We should probably play a follow up ticket to this to allow you to > configure the prefix/suffix of your variables as well. > > > > John > > > > On Fri, Apr 15, 2016 at 6:31 AM Mark Struberg <strub...@yahoo.de.invalid> > wrote: > > Hi! > > > > I recently committed a feature to evaluate variables in config values. > > > > > > Basically having something like: > > ----------------------------------------------------------------- > > document.server.url=http://localhost:8081 > > myapp.document.lists=${document.server.url}/docapp/list > > myapp.document.admin=${document.server.url}/docadmin/app > > ----------------------------------------------------------------- > > (atm adding this to our docs as well) > > > > > > > > In @ConfigProperty I enabled this feature by default. > > > > In ConfigResolver I just added a method 'evaluateVariables' in > TypedResolver and > > added new method to ConfigResolver itself: > > > > public static String getPropertyValue(String key, String defaultValue, > boolean evaluateVariables) > > > > > > How should the other getPropertyValue etc methods in ConfigReolver > behave? > > > > Thinking about it for a few days now I think it would be ok to enable > this feature for them as well. > > > > wdyt? > > > > LieGrue, > > strub > >
