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

Allow optional ConfigMapping #845

Open
marcelstoer opened this issue Nov 17, 2022 · 13 comments
Open

Allow optional ConfigMapping #845

marcelstoer opened this issue Nov 17, 2022 · 13 comments
Labels
enhancement New feature or request

Comments

@marcelstoer
Copy link
Contributor

Origin: https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/.E2.9C.94.20Make.20a.20whole.20.40configmapping.20interface.20optional.

In some cases it would be really helpful to declare a whole @ConfigMapping optional e.g. through @ConfigMapping(prefix = "my-app.my-component", optional = true). You could use this to make the existence/availability of a whole sub-tree of config properties conditional. When you inject the config mapping class into a bean it would then use Optional<MyComponentConfig> myComponentConfig rather than MyComponentConfig myComponentConfig.

Maybe...the most simple implementation would be to just not throw an exception if you don't find the property my-app.my-component.foo.bar if one of the segments refers to an optional @ConfigMapping (my-component in this example).

P.S. would be a great companion for #844 😄

@renegrob
Copy link

renegrob commented Nov 17, 2022

👍🏼 Often there's a configuration like

my-service:
   my-subsystem:
      prop1: value1
      prop2: value2
      prop3: value3            

And the configuration of my-subsystem is either completely mandatory or completely optional.

The following configuration example requires to define everything on the level my-service which means that it is impossible to use libraries that contribute to the project's configuration and forces me to have a single model of the complete configuration.

@ConfigMapping(prefix = "my-service")
interface MyService {
   Optional<MySubSystem> mySubsystem();
}

There should be a way to define such configuration mappings in libraries that are used in multiple projects without forcing them to be root configuration mappings. Quarkus has many such cases but Quarkus (e.g. all extensions share quarkus.). But extensions use a different configuration definition model.

@radcortez
Copy link
Member

Maybe...the most simple implementation would be to just not throw an exception if you don't find the property my-app.my-component.foo.bar if one of the segments refers to an optional @ConfigMapping (my-component in this example).

If the entire my-component subtree is wrapped as an Optional inside the mapping it shouldn't throw an exception.

There should be a way to define such configuration mappings in libraries that are used in multiple projects without forcing them to be root configuration mappings. Quarkus has many such cases but Quarkus (e.g. all extensions share quarkus.). But extensions use a different configuration definition model.

Is this because of the validation that all subtrees must be mapped that is forcing you to have a single root tree with all sub-elements, even if optional?

@marcelstoer
Copy link
Contributor Author

If the entire my-component subtree is wrapped as an Optional inside the mapping it shouldn't throw an exception.

That's the standard case where you have a (single) @ConfigMapping class for the entire application. That works. However, what I tend to do is 1 subsystem/component = 1 Maven module = 1 @ConfigMapping (per module). Hence, my applications have usually as many @ConfigMapping classes as there are subsystems/components.

@dmlloyd
Copy link
Contributor

dmlloyd commented Nov 18, 2022

I think it makes sense in that if a group can be optional then a root should be able to be optional too, using the same (conceptual) logic. But there would have to be additional API to get a config mapping optionally e.g. Optional<MyService> optMyService = config.getOptionalConfigMapping(MyService.class), and maybe some other support as well. I fear that this might not be very easy to implement though.

@radcortez
Copy link
Member

I have to say that I don't see much difference between having:

@Inject
Optional<MyMapping> mapping

interface MyMapping {
  Nested nested();

  interface Nested {
  }
}

(not supported)

or

@Inject
MyMapping mapping

interface MyMapping {
  Optional<Nested> nested();

  interface Nested {
  }
}

(supported)

When having multiple modules, each uses a mapping class, and each mapping class can map a single optional nested element for the subtree. Wouldn't that work?

@renegrob
Copy link

I agree that in your example there isn't much difference.
However in this case:

@Inject
Optional<MyMapping> mapping

interface MyMapping {
  String: prop1();
  String: prop2();
  String: prop3();
}

vs

@Inject
MyMapping mapping

interface MyMapping {
  Optional<String> prop1();
  Optional<String> prop2();
  Optional<String> prop3();
}

There's a big difference: The first implementation validates that either all or none of the String values are present, while the 2nd implementation requires me to do the whole validation and mapping logic in the code. The 2nd implementation is like injecting the values using @ConfigProperty.

@radcortez
Copy link
Member

Ok, let me see what I can do.

@HerrDerb
Copy link

Any news about that?

@radcortez
Copy link
Member

No progress at the moment. Unfortunately, we are in the process of moving the main branches to jakarta and updating to Jakarta 10, so that is slowing us down on the enhancements side.

Hopefully, we should be done in the next few days so we can start moving with features.

@manofthepeace
Copy link
Contributor

Would be really nice to have such feature. I have a case where I have a pretty complex configMapping. If a user does not configure any of it i'd like to act as the feature was disabled but I cannot without having everything optional inside, which is not right as things are required when feature needs to be enabled. I thought I could get away with this with a ConfigurableConfigSourceFactory but this fails as the configMapping expects values

@radcortez
Copy link
Member

Did you try #845 (comment)?

@manofthepeace
Copy link
Contributor

Yes I already use that pattern. My Problem goes a bit like this; #845 (comment) because I have multiple nested interfaces.

@radcortez
Copy link
Member

Could you share your mapping classes? I think it would be easier if we looked at real examples. Thanks!

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

No branches or pull requests

6 participants