avaje-config icon indicating copy to clipboard operation
avaje-config copied to clipboard

Add URI Loader SPI

Open SentryMan opened this issue 1 year ago • 84 comments

  • Adds a URI loader SPI for loading config from custom URIs
  • delete the Parser Class (it was basically just a map)

this is what I got so far, what do you think @agentgt?

Resolves #175

SentryMan avatar Sep 20 '24 19:09 SentryMan

Let me follow up and inspect more on Monday. It is looking good though!

agentgt avatar Sep 21 '24 02:09 agentgt

Hmmm, not enthused by this PR. Let's start from the very top with the motivation. What are some actual examples of a URIConfigLoader? I've seen some things like env:// but that is very unconvincing to me because avaje-config already handles environment variables, so what exactly are some URIConfigLoader implementations that are intended to exist?

rob-bygrave avatar Oct 09 '24 02:10 rob-bygrave

I talk about some of the URI we use here : https://github.com/avaje/avaje-config/issues/175#issuecomment-2363744845

ultimately it is going to depend how opinionated you want to keep ajave.

The initial loading of avaje can all be expressed in URI. We do special loading:

  • system properties
  • env variables
  • profiles
  • some default resources
  • etc

In our config framework only one classpath resource is loaded and all that other shit is URIs for load.properties this includes cloud resources and k8s.

I’ll add more tomorrow.

agentgt avatar Oct 09 '24 03:10 agentgt

this includes cloud resources and k8s. I'll add more tomorrow.

Thanks. Yeah, I'm wanting to know the specific things that we are trying to add here. It isn't clear to me if we are looking to do some existing stuff (env vars, system properties, profiles) differently. For example already have avaje-aws-appconfig for AWS AppConfig. Good to understand clearly where this is heading to, is this looking to replace the existing default loading etc.

rbygrave avatar Oct 09 '24 05:10 rbygrave

CLASSPATH, FILE, ENV, CMD,

avaje-config already has built in support for these with defined ordering.

JAR,

A Jar specific URI resource I'm guessing.

HTTP, HTTPS, STDIN, stdin:///?mimeType=

I see, these things are all effectively supplying an InputStream or a Reader under the hood. No cloud specific library dependency etc.

rbygrave avatar Oct 09 '24 07:10 rbygrave

still early for me but want to catch you while your available.

CLASSPATH, FILE, ENV, CMD,

avaje-config already has built in support for these with defined ordering.

No it is not well defined. We just found issues with load properties not chaining.

Ultimately the ordering of resource or property loading is incredibly important. Correct?

With load properties we can configure all of that and I rather avaje not try to sniff around adding things to the final key value map unless I tell it to… ideally through config.

This idea of recursively loading resources based on a list of resources is a powerful model that makes core less likely to change. Currently I see core changing all the time including random things like: https://github.com/avaje/avaje-config/blob/master/avaje-config/src/main/java/io/avaje/config/DockerHost.java

Furthermore besides order sometimes we need to pass options for the resources and URI allows this. Things like name transformation.

JAR,

A Jar specific URI resource I'm guessing.

HTTP, HTTPS, STDIN, stdin:///?mimeType=

I see, these things are all effectively supplying an InputStream or a Reader under the hood. No cloud specific library dependency etc.

Those are the builtin ones that are in core. The cloud ones are a separate modules.

there is a k8s one, vault, google cloud etc.

that Amazon one you guys have should be a URIConfigLoader.

Then I can decide what order (precedence) it has and pass other config if needed.

I’ll be more specific in a couple of hours when I’m fully up.

agentgt avatar Oct 09 '24 09:10 agentgt

that Amazon one you guys have should be a URIConfigLoader

That AWS one is reloadable [and I expect all the remote ones to often be reloadable]. Are URIConfigLoader reloadable? This API in this PR doesn't look like it, so something doesn't look right there?

I rather avaje not try to sniff around adding things to the final key value map unless I tell it to… ideally through config

Right, I was getting that vibe. I feel that means we need to see if we can "see a path forward" for the overlap that already exists like ENV, System properties, classpath resources, files, command line args. That is, it would be silly to end up with duplicate logic for each of these things right? And in the "complete control over all loading case" we are ideally wanting all of those available via URIConfigLoader ...

For classpath resources, files, command line args my gut is saying that there wouldn't be much issue there. I'm more uncertain about ENV and System properties. Do you have a good definition of how those work as URIConfigLoader so that we can understand how that is different to avaje-config today.

rbygrave avatar Oct 09 '24 11:10 rbygrave

That AWS one is reloadable [and I expect all the remote ones to often be reloadable]. Are URIConfigLoader reloadable? This API in this PR doesn't look like it, so something doesn't look right there?

I had asked at one point how many use the reloading feature as we no longer really use it (our version). The reality is cloud actually has kind of killed reload. I mean reload while running. This is especially so if you are using GitOps.

That is config doesn't change during runtime and if it does the k8s cluster basically reloads.

The old school way of having config reloading was largely when reboot was expensive and that was when we used to do it. Netflix used to do it as well as Hystrix needed config changes ( ( Archaius ) now I doubt they use dynamic config but maybe they still do.

Anyway we used to track URI resources and they would have flags on whether they could be reloaded but ultimately what we ended up doing after many years is reload the entire thing with a backup of System.properties. You externally tell the whole thing to reload. This is because it gets very confusing if you reload part of the configuration graph. Also most things just cannot respond to dynamic configuration particularly while avoiding coupling to a configuration framework. Show me third party libraries besides Rainbow Gum that do. It requires mutability or a complete context refresh which is almost a reboot.

I assume load.properties never supported reload?

Also there is no order to ConfigurationSource. I assume we just pick one? Do we allow multiple ones? What happens if we deploy to multiple clouds?

Let me give you an example of one our configurations. It is important to note that unlike other key values configuration system ours is order aware (e.g. more like: List<Entry<String,String>> than Map. The final resolution can generate a Map though). Also snaphop-kvs has the concept of "variables" which are different than properties. They are basically properties that don't end up in the final configuration used for interpolation.

# classpath://system.properties <- this is analagous to app.properties.
app.package=com.mycompany.myapp
# snaphop-kvs instead of a single property for chaining like load.properties
# uses multiple meta properties that have prefix
# _require is like load properties but the resources has to be there
# _maybe is another one and it is basically load.properties
_maybe_env_env=env://MY_COMPANY_/var.MY_COMPANY_
_require_shared=classpath://app-group-share.properties

_require_env=env://MY_COMPANY_// 
_require_system=system:/// 

In app-group-share.properties

# we do our standard config here.
# notice order matter here again.
app.group=mycompany
_profile=${var.MY_COMPANY_PROFILE}
## note here profile is the URI scheme which is a plugin that will take
## the variable `_profile`, split and replace `__profile__` in the URI.
## I dont think we need this for avaje-config but just showing what we do.
_maybe_profile=profile:classpath://app-group-share-__profile__.properties

SO if someone does

export MY_COMPANY_PROFILE=canary,aws

We load app-group-share-canary.properties and app-group-share-aws.properties.

# app-group-share-aws.properties

_require_aws=aws://?some_parameter=xyz

You see how I was able to selectively decide whether or not to use Amazon with config.

The above may not be helpful to you but I wanted to show you how we configure configuration through configuration.

See our configuration is modularized such that loading is distinctly separate from Config.getProperty and that loading happens very early.

Our core module basically is using pseudo code:

// there are tons of ways to do recursion so take this lightly
public void load(List<KeyValuesResource> resources, SomeRecursionState state) {
  for (var r : resources) {
    var kvs = loadSingle(r.uri())
    state.addKeyValues(kvs);
    var resources = loadPropertiesLikeToKeyValuesResources(kvs);
    // recurse
    load(resources, state);
  }
}

That is all core does. It loads a list of Key Values by recursively loading URIs from those Key Values. Besides parsers the only extension point is URI -> KeyValues (there are more arguments like environment and variables but that is the gist).

Besides a core module we ship an opinionated module. This sort of does what avaje-config does by default.

// SPI loads the following when you call `Config.getSomething` akin to `LogFactory.getLogger(...)`
KeyValues kvs = KeyValuesSystem.load(KeyValuesResource.ofURI("classpath://app.properties");
// set kvs to system properties or not
// returns as some `Config` object which is more like `Map`.

Anyway I agree the current URI loading in avaje-config needs some more thinking. I think there are some confusing things that need to be formalized like if load.properties gets to play the reload game.

Regardless I think the core of avaje should be more core and the opinionated loading should be separated. The idea is the core of a library should not be changing much and represents fundamentally how it works. Right now that is not clear given load.properties chaining.

agentgt avatar Oct 09 '24 13:10 agentgt

As for the URI of env and system it allows loading those guys up but picking which keys and if you want the keys prefixed.

Honestly that part I did not formalize well but there is like a

EnvKeyValuesResource and a SystemKeyValuesResource if you wanted to do it programmatically. I never got to formalize standard meta uri query parameters.

agentgt avatar Oct 09 '24 14:10 agentgt

The big thing selling me is the ability to define the order of loading via load.properties, or even whether to load at all. Something we currently have no control over with our current spis. I currently need to write separate logic in my ConfigurationSource so that it doesn't activate during tests and throw an exception.

SentryMan avatar Oct 09 '24 14:10 SentryMan

@SentryMan Yeah my current concern is that the core model is not flexible enough and there are all these "special" cases. I learned that the hard way.

I think what I need to do is opensource at least this core part of our system to show you and @rob-bygrave the modeling choice we picked.

I was avoiding this and doing PRs for a very long time because I felt our system was perhaps too complex and I did not want to poison avaje-config into helidon config or archaius.

I'm now rethinking this.

I have had some notes on modeling here: https://github.com/avaje/avaje-config/pull/180#issuecomment-2402628990

I have omitted things hence why I think putting some javadoc core of our stuff will give a better cohesive picture.

Going back to reloading...

Helidon and most config systems do not allow chaining or resources to load resources and have priority ordering of resources and use a tree instead of a giant list but lets put that aside cause I prefer our mutual chain approach.

However if you guys really want hot reloading of individual resources then the chain approach will not work well and priority tree will (the tree structure cannot change after load).

Otherwise interpolation on constant reloading can get very confusing and in some cases system concating can happen. I'll give an example later but this is why a chain requires a full load every time something changes and/or caching. Just Imagine if you add more load.properties to somewhere on the chain and that gets dynamically loaded.

agentgt avatar Oct 09 '24 16:10 agentgt

I have no opinion on reloading, I don't tend to reload configs that often

SentryMan avatar Oct 09 '24 16:10 SentryMan

I assume load.properties never supported reload?

99% Correct. The caveat is that load.properties can load file resources, and we can enable reloading of file resources.

reloading

I'd suggest the 99% use case is dynamic feature toggle enable/disable.

want hot reloading of individual resources then the chain approach will not work well

In practice I'd say it does work fine, in that the "reload" effectively only reloads changed properties and these are 99% feature toggle enable/disable. A reload is perhaps better described as "polling for changed properties ... for the purpose of feature toggle enable/disable, logging level change etc". There is no support or intention for adding sources to the chain as part of a "reload".

A reload in this sense isn't or doesn't need to be thought of as a "Refresh the entire chain of property sources". It is more a "Poor mans feature toggling system".

In this sense, I'm comfortable that a chain of resources is still a good approach and that it is still fine to support the "reloading/polling for changed properties" of selected sources in the way that it is used and how it is expected to work with avaje-config now.

The design point to note around reloading is it is in io.avaje.config.ConfigurationSource, with the built-in addition that we can explicitly enable reloading file resources [useful with k8s].

had some notes on modeling here

Yes, appreciate that, gives us a better sense of the design issues and differences etc.

ours is order aware (e.g. more like: List<Entry<String,String>> than Map

Design wise that is compatible with what avaje-config is trying to do. I can see that your config system has better control and better ordering. Lots of good ideas here.

rbygrave avatar Oct 09 '24 22:10 rbygrave

load.properties ... _require _maybe

Got it. Sorta like a load.properties=classpath://app-group-share.properties?required

system:///

I'm thinking this loads all system properties?

env://MY_COMPANY_//

Can you explain what this does? Does it load all the ENV vars that start with MY_COMPANY_ ?


separate logic in my ConfigurationSource so that it doesn't activate during tests

application-test.properties will load before ConfigurationSource, so my expectation is that there is a myloader.enabled=false entry in application-test.properties. Each ConfigurationSource implementation just has a configuration.enabled("myloader.enabled", true) check [because the vast majority of the time we don't want these sources when we are running tests].

rbygrave avatar Oct 09 '24 23:10 rbygrave

Each ConfigurationSource implementation just has a configuration.enabled("myloader.enabled", true) check [because the vast majority of the time we don't want these sources when we are running tests].

Precisely the problem, I have to duplicate this logic for every configuration source I write and maintain a unique enable key. What we got here is an opportunity to make it so custom configuration is activated only when specified via config instead of needing the overhead of having that extra code.

There is also the matter of how there is currently no easy method to control the ordering.

SentryMan avatar Oct 10 '24 00:10 SentryMan

duplicate this logic for every configuration source I write and maintain a unique enable key

How many have you got? What do they do?

rbygrave avatar Oct 10 '24 01:10 rbygrave

opportunity

Yes. In my mind, with some insights forming here it looks like we have "Over cooked the Turkey" in a few ways and Adam has kindly provided us a view into an alternative approach that has a whole bunch of benefits.

Currently I'd kind of summarise it as:

  • Use a src/main/resource/application.properties ... as starting point with some properties and a load.properties
  • Use recursive load.properties= ... mechanism with ordered "URI based loaders" to produce ~ List<Entry<String,String>> [approximately what avaje-config is already doing]
  • Effectively look to remove/replace the predefined load ordering, might look to merge ConfigurationSource into "URI based loader"

Adams examples above were very useful for me. I'm thinking that it would be worth producing a prototypical example like:

src/main/resources/application.properties

hello.world=hi
load.properties= cmd://-P/ 
  env://MY_PREFIX_/ 
  system:// 
  classpath://app-shared.properties
  profile:classpath://app-profile-example-${profile}.properties
  file://${user.home}/myapp/local.properties 
  file://my-app.yaml 
  aws://?some_parameter=xyz

src/test/resources/application-test.properties

## disable some stuff when testing ...
metrics.reporting.enabled=false
config.disableUris=aws

rbygrave avatar Oct 10 '24 02:10 rbygrave

How many have you got? What do they do?

Some for vault and some for aws secret manager. It's not a huge problem to add the disable logic, but it's a bit of an inconvenience.

SentryMan avatar Oct 10 '24 03:10 SentryMan

Looking again through the PR, I feel the API could have been more like:

public interface URILoadContext {

   Optional<ConfigParser> parser(String extension);

   // potentially need other methods like enabled() and schedule() below
   // if it is to support replacement/unification with ConfigurationSource 

   boolean enabled(String key, boolean enabledDefault); // disable me when running tests, might be a better way to do this

   String get(String key, String defaultValue); // read config needed like aws region etc, could be URI query params instead?

   void schedule(long delayMillis, long periodMillis, Runnable runnable); // register polling for feature toggle changes etc
}

public interface URIConfigLoader extends ConfigExtension {

  /** Used redacted URI as the source for the entries, redact / excluding query path etc */
  String redact(URI uri);

  String supportedScheme();

  Map<String, String> load(URI uri, URILoadContext loadContext);
}

So thinking, rather than a PR trying to integrate this, I'm more thinking along the lines of implementing URIConfigLoader for resource, file, env, system, cmd. Then look to create a completely new "pure uri based" loading mechanism. Then after that look at the integration options. That is, go for a distinctly separate loading approach first. Then after that review it and look at integration options - we might even consider this as a more clean room distinctly separate loading approach [if it isn't too big and it looks nicer/cleaner that way we might be tempted by that].

That is, I'm feeling it might be useful to defer the "how to integrate this into existing behaviour" thinking until we see how clean / nice it is standing all by itself. We might see if that can replace/unify with ConfigurationSource.

Edit: To be clear, there is a chance that we take a path where we have a "new loading mechanism" and we sunset the "old loading mechanism". Something like main/resources/application.properties contains a new key like uri.config.loading= <uris> ... and if that is present it purely uses the "new loading mechanism". It could be we end up there depending on how this prototypes out.

rbygrave avatar Oct 10 '24 03:10 rbygrave

merge with ConfigurationSource

Not sure about that, while they both load properties, uri loading seems to work more like a custom parser, with uris instead of files types.

Enabled

No need for enabled/disabled methods since uri loading is opt-in and based on load.properties

SentryMan avatar Oct 10 '24 14:10 SentryMan

@rbygrave

Can you explain what this does? Does it load all the ENV vars that start with MY_COMPANY_ ?

Yes what env:// and system:// do is load key words from environment and system properties respectively.

Both have a sort of mini DSL in their paths and query parameters to transform and filter by name. This came about organically and I'm still not entirely sure its syntax is the right choice because of consistency.

Now to actually answer your specific question of: env://MY_COMPANY_//

That will load all environment variables starting with MY_COMPANY and remove the prefix of MY_COMPANY_.

The env URI will also create a normalized dotted property from environment variables where effectively _ is turned into . because . are not allowed in env variables.

So having the environment variable would create the following to key values.

MY_COMPANY_SOME_FLAG -> {SOME_FLAG, some.flag}

Essentially the format is akin to vim's or sed %s/find/replace/ however it is doing double duty of selecting and transforming. Also I really should have used regex instead of the whole prefix stuff.

Also I bet you thought it would just select the variables and not remove the prefix. So you can see it is confusing.

BTW if you did want to keep the prefix it would have been

env://MY_COMPANY_/MY_COMPANY_

There are lots of problems with the above syntax. For one other URI things can't use it because URI.getPath is actually being used e.g. file and classpath. The other is regex would have been more clear. You also can't easily select on env variables and have them prefixed.

Basically the problem is that there is a in theory kind of global need to transform key values particularly their names. I wasn't sure to either:

  • Make it case by case for each URI. Consistency and code reuse be damn.
  • Have some sort of abstract class where your URI can follow some form of the DSL formalized. Maybe query parameters that are automatically removed and we do something like this in some places. This is akin to what you did with the ?required however we prefix those meta parameters with _.
  • Have another mechanism do the filter/transform through some meta like key values (_filter_key_name_uri_to_load=filter_name)

Somewhat related and is a little messed up is that our core KeyValue can have parameters extracted from it by the key name and its value.

KeyValue kv =  KeyValue.of("_require_stuff?key=blah", "classpath://b.properties?_anotherKey=foo";
KeyValues parameters = kv.keyParameters();
assertEquals(parameters.toMap(), Map.of("key", "blah"));

To show that in properties

# notice backslash
_require_stuff?key\=blah=classpath://b.properties?_anotherKey=foo

The above is pretty nasty and does not really work for a single key load.properties world. This was all in some vain to figure out how to pass properties to the uri loader (although key parameters does have some uses for other things).

So what we do now is use Variables which is just a Function<String,String> that gets passed to the URI loader. The extensions can use those instead of uri query parameters and variables is chainable so you can combine query parameters and variables.

The Variables is usually an already interpolated map of essentially the key values that have already been loaded plus other things.

Also at any time you can reinterpolate a KeyValues.

KeyValues kvs = ...;
Map<String,String> resolved = kvs.interpolate(variables);

Sorry the above is kind of an unorganized brain dump. I'll see if I can open source our stuff soon or at least the interfaces with javadoc.

agentgt avatar Oct 10 '24 14:10 agentgt

Looking again through the PR, I feel the API could have been more like:

public interface URILoadContext {

   Optional<ConfigParser> parser(String extension);

   // potentially need other methods like enabled() and schedule() below
   // if it is to support replacement/unification with ConfigurationSource 

   boolean enabled(String key, boolean enabledDefault); // disable me when running tests, might be a better way to do this

   String get(String key, String defaultValue); // read config needed like aws region etc, could be URI query params instead?

   void schedule(long delayMillis, long periodMillis, Runnable runnable); // register polling for feature toggle changes etc
}

public interface URIConfigLoader extends ConfigExtension {

  /** Used redacted URI as the source for the entries, redact / excluding query path etc */
  String redact(URI uri);

  String supportedScheme();

  Map<String, String> load(URI uri, URILoadContext loadContext);
}

Yes the URI loader contract in our implementation looks more like this. I purposely had not shown it to @SentryMan.

Our is even a tad more generic as it doesn't need supported schemes but instead does the whoever responds first to this URI ... e.g. loop till not null.

public interface KeyValuesLoaderService {

	public @Nullable KeyValuesLoader findLoader(
			KeyValuesEnvironment environment,
			KeyValuesResourceContext resourceInfo)
			throws IOException,
			KeyValuesNotFoundException;

}

The "Context" has the URI and lots of other stuff like parsers etc. KeyValuesLoader is a supplier of KeyValues that throws some checked exceptions. In hindsight I can't recall why I didn't just eagerly return or allow the lazy load.

agentgt avatar Oct 10 '24 15:10 agentgt

@rbygrave @SentryMan

I forgot to add on the unit testing approach. We used to have our default (opinionated loading) do something like:

Variables variables = Variables.of(System::getProperty);
// just replace system.properties with application.properties or whatever default
var kvs = KeyValuesSystem.load(List.of(KeyValuesSource.require("classpath:/system.properties"), KeyValuesSource.maybe("classpath:/test-system.properties"), variables);

(the above is not the exact API as there are some builders and shit).

But now we just do classpath:/system.properties.

Then application modules (the ones with static void main(String args[])) would put system.properties in their src/main/resources and sub modules (e.g. some repository layer) put it in src/test/resources.

With this setup we rarely need test-system.properties but if you did you do:

# system.properties
_maybe_test=classpath:/test-system.properties

This works because maven isolates the class/modulepaths but other build systems might have issue with the above.

Also I kind of skimmed over how you accomplish profiles. Profiles is the hardest URIConfigLoader because the question is how does it load sub resources.... I'll give you a hint using your API:

  Map<String, String> load(URI uri, URILoadContext loadContext) {
    var profiles = Stream.of(loadContext.get("_profiles").split(",")).filter(..)...; // get list of profiles
    var uriString = uri.toString().substring("profile:".length());
    var loadPropertiesValues = profiles
      .map(p -> uriString.replace("__PROFILE__",p)
      .collect(Collectors.joining("\n"));
    // here is the trick.
    return Map.of("load.properties", loadPropertiesValue);
  }

Then in system.properties:

_profiles=dev,test # or fetch from something else
load.properties=profile:classpath:/__PROFILE__-system.properties

Which would maybe load dev-system.properties and test-system.properties

agentgt avatar Oct 10 '24 17:10 agentgt

how you accomplish profiles

cool, I made another PR for that to make it general purpose

To be clear, there is a chance that we take a path where we have a "new loading mechanism" and we sunset the "old loading mechanism"

I was always thinking of this feature more like a specialized ConfigParser, than a replacement for ConfigurationSource given how it accepts data in the form of a URI to get a k/v map. So I guess I'm not seeing what benefit rewriting the existing loading mechanism buys us.

If we replace our current loading mechanism with URI loading, the only ones we can really use in core avaje-config would be for classpath and file. If you notice, we only check those other locations like system/env/cmd for more files to load, or some specific properties (POD_NAME, and such).

We wouldn't really get any use out of env:///system:///cmd:// loaders in the core. And it would lock out anyone that wants custom handling, such as how @agentgt has his own wacky logic for transforming env variables. (you can only have one loader per uri scheme)

SentryMan avatar Oct 14 '24 03:10 SentryMan

more like a specialized ConfigParser

I reckon its a "Source". Adams API has it as a KeyValuesLoader and we've got a URIConfigLoader which can have "parameters" (from uri path or query parameters) but it's job is to actually load key/value pairs as configuration source (e.g. stdin).

it accepts data in the form of a URI to get a k/v map

Those are parameter values that the URIConfigLoader can use to configure how it loads the data. Like aws region, or mimetype, or env prefix trimming. Not the actual data per say but parameters that the loader can use.

We wouldn't really get any use out of env:// system:/

It depends. Yes, we literally might not include those in favor of just continuing the approach in avaje-config where they are just part of the eval. For myself I feel like the avaje-config approach is simpler here.

cmd://

The advantage of having this as a URIConfigLoader is that it is then explicit, can be parameterised (currently we have fixed -P -p prefixes), gets explicit ordering relative to other URIConfigLoader's [where as currently the ordering in avaje-config is implicit and can't be controlled explicitly].

you can only have one loader per uri scheme

Yes but the loader can have different behaviour based on the URI path or parameters. I'm sure its used this way.

logic for transforming env variables

In my mind the "trick" is to distill / remove all the extraneous features that don't justify themselves. For example, currently we wouldn't trim ENV variables but instead just have them eval and use forPath(prefix), and the ONLY transform on keys we do is for ENV names translating between SOME_FLAG and some.flag which is part of eval. So there is this difference in approaches here but maybe that distills down to ... just use the existing eval and NOT do env:// or system:// because it's already covered pretty well by eval.

rbygrave avatar Oct 14 '24 04:10 rbygrave

there is a in theory kind of global need to transform key values particularly their names.

Currently with avaje-config the only transform on key values is that the eval also supports ENV variables and so automatically looks for some.flag and SOME_FLAG etc. The trimming of keys is handled via configuration.forPath(prefix). This approach means that there isn't any general need for key transform per say.

rbygrave avatar Oct 14 '24 04:10 rbygrave

cmd://

The advantage of having this as a URIConfigLoader is that it is then...

Yeah I know how it can be useful, I'm more saying that by adding it to the core we might force things to be a certain way that we might regret later.

Yes but the loader can have different behaviour based on the URI path or parameters. I'm sure its used this way.

Indeed, this is why I'm saying it would be better to leave things as flexible as possible and not define a set behavior for specific schemes. (Aside from resource and files of course)

So there is this difference in approaches here but maybe that distills down to ... just use the existing eval and NOT do env:// or system:// because it's already covered pretty well by eval.

As I understand it, @agentgt has env variables that he wants to transform a certain way that can't be done with that approach.

SentryMan avatar Oct 14 '24 05:10 SentryMan

@rbygrave what would you have me do?

SentryMan avatar Oct 14 '24 13:10 SentryMan

@rbygrave what would you have me do?

Honestly I'd recommend we "pause" the PR / Implementation until we get some really good understanding of the options. That is, there is some danger is too quickly going into "solution" / "implementation" mode and I think there is quite a lot more thinking and understanding to be done.

rbygrave avatar Oct 15 '24 06:10 rbygrave

Example case: Let say we want to load something via profiles, cmd line args, and the AWS plugin:

config.sources=profiles://classpath:application-__PROFILE__.properties cmd://-P/ aws://

Now the aws plugin actually needs some parameters so we might specify the parameters it needs in URI form like:

config.sources=profiles://classpath:application-__PROFILE__.properties cmd://-P/ aws://?region=${AWS_REGION}&appName=myapp&env=${ENV:local}

But the config.sources line can start looking unmanageable so we instead do something more like:

aws.appconfig.region=${AWS_REGION}
aws.appconfig.appName=myapp
aws.appconfig.env=${ENV:local}

config.sources=profiles://classpath:application-__PROFILE__.properties cmd://-P/ aws://

And we might do something similar for profiles like:

aws.appconfig.region=${AWS_REGION:ap-southeast2}
aws.appconfig.appName=myapp
aws.appconfig.env=${ENV:local}

profiles.source=classpath:application-${profile}.properties

config.sources=profiles:${profiles.source} cmd://-P/ aws://

Then if we have complex cases, like say profiles has both classpath and file sources:

aws.appconfig.region=${AWS_REGION}
aws.appconfig.appName=myapp
aws.appconfig.env=${ENV:local}

profiles.source=classpath:application-${profile}.properties file:${profile}-config.yaml

config.sources=profiles://${profiles.source} cmd://-P aws://

Lets say we have decent defaults like -P for cmd etc, and lets also add a vault plugin. It could be more like:

aws.appconfig.region=${AWS_REGION}
aws.appconfig.appName=myapp
aws.appconfig.env=${ENV:local}

myvault.prefix.param0=foo
myvault.prefix.param1=bar

config.profiles.source=classpath:application-${profile}.properties file:${profile}-config.yaml

config.sources=profiles cmd aws vault://myvault.prefix

rbygrave avatar Oct 15 '24 08:10 rbygrave