Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support subsets from a Config #981

Open
gastaldi opened this issue Aug 23, 2023 · 6 comments · May be fixed by #983
Open

Support subsets from a Config #981

gastaldi opened this issue Aug 23, 2023 · 6 comments · May be fixed by #983
Labels
enhancement New feature or request

Comments

@gastaldi
Copy link
Contributor

Because the Config interface has a nice way to convert values and handle optional values, it would be nice if the API allowed to extract a subset of a Config to pass it as a parameter to methods that don't care about the external structure:

Eg.

app.foo.username=guest
app.foo.password=guest
SmallRyeConfig config = ...
Config subset = config.subset("app.foo");
// Now I can call methods taking the config as a parameter that only cares about the leaf nodes:
configure(subset);

public void configure(Config subset) {
   String username = subset.getValue("username",String.class); // This would return "guest"
}
@dmlloyd
Copy link
Contributor

dmlloyd commented Aug 24, 2023

I think this is a good feature. The current internal APIs are not very friendly to this kind of approach though. It might be worth shaking things up internally because the same restrictions that make this feature hard to implement also make the config mapper very complex to implement, so addressing this might pay off in multiple areas.

gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
@gastaldi gastaldi linked a pull request Aug 24, 2023 that will close this issue
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
gastaldi added a commit to gastaldi/smallrye-config that referenced this issue Aug 24, 2023
@radcortez
Copy link
Member

Thanks for the work @gastaldi

This is a very valid feature, and we should have it. On the other hand, I think we lose a lot of its potential by returning a Config instead of SmallRyeConfig.

Yes, right now, we cannot do that, but I'm also thinking for a while that SmallRyeConfig should become an interface. The MP Config API will not get any new updates, and we have to rely on many useful APIs we added in SmallRyeConfig directly. Some other projects like reactive messaging already have to reimplement their ownConfig version to handle Kafka config system.

I know that these can happen separately, but how about if we do this work here too? Or, you don't think we need to separate the SmallRyeConfig API @dmlloyd ?

@gastaldi
Copy link
Contributor Author

Having SmallRyeConfig as an interface makes more sense I think. And given we have SmallRyeBuilder as the factory for this object (and not being instantiated directly), it shouldn't (hopefully) break existing code.

We could return a SmallRyeConfig by introducing a subclass with a String prefix and prepending that to the getValue/getOptionalValue calls if not null, but separating into a different delegating implementation is cleaner, so +1 to make SmallRyeConfig an interface.

@radcortez
Copy link
Member

Do you want to have fun with it? Or do you prefer to do it as a separate PR? :)

@gastaldi
Copy link
Contributor Author

gastaldi commented Sep 1, 2023

I'll add to my never-ending TODO list :) Better make it as a separate PR to make sure we don't break anything ;)

@dmlloyd
Copy link
Contributor

dmlloyd commented Sep 1, 2023

If we're not immediately planning on breaking compatibility, I would recommend adding a new interface which is implemented by SmallRyeConfig (and which might or might not extend Config) rather than changing the class to an interface which will break all existing code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants