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

Add the ability to group configuration into a class/interface #279

Closed
kenfinnigan opened this issue Apr 16, 2020 · 11 comments
Closed

Add the ability to group configuration into a class/interface #279

kenfinnigan opened this issue Apr 16, 2020 · 11 comments
Labels
enhancement New feature or request
Milestone

Comments

@kenfinnigan
Copy link
Contributor

Quarkus introduced @ConfigProperties (https://github.com/quarkusio/quarkus/blob/master/extensions/arc/runtime/src/main/java/io/quarkus/arc/config/ConfigProperties.java) as a way to mark all fields on a class/interface as having a common prefix.

I think it would be a good feature to have within SmallRye Config as well.

It would likely simplify a lot of configuration usage in SmallRye projects

@kenfinnigan kenfinnigan added the enhancement New feature or request label Apr 16, 2020
@radcortez
Copy link
Member

Yes, I thought about this as well. Let us flush the interceptor work into a new release. We probably want to complete #278, before doing the next release.

And, then move with #272 to separate the CDI implementation. I think we should do it in a separate release and then work on this :)

@radcortez radcortez added this to the 1.8 milestone Apr 16, 2020
@kenfinnigan
Copy link
Contributor Author

Timing isn't a concern from my perspective, it was more that it was an idea/note I'd had from a meeting ages ago and wanted to add it so it can be planned.

@dmlloyd
Copy link
Contributor

dmlloyd commented Apr 17, 2020

We might be able to take it a bit farther tbh. For some of my other work I'm needing a more powerful configuration system, so it might be time to create something for complex configurations on SmallRye that we can move to (and maybe Quarkus can move there as well in the future).

@kenfinnigan
Copy link
Contributor Author

+1 to that

@radcortez
Copy link
Member

@dmlloyd let me know some of your ideas and I might be able to help you implement it :)

@dmlloyd
Copy link
Contributor

dmlloyd commented Apr 17, 2020

I'm still forming up my ideas. But one thing I'm sure of is that I have a lot of regret using fields in Quarkus instead of interfaces.

My other ideas so far:

  • This shouldn't be a CDI-only feature; we should be able to set the configuration interfaces on the config builder, and then be able to get the instances from the constructed configuration at will
  • Injecting single config properties in CDI seems to have been a design error. I'd want to consider the possibility of eventually deprecating the @ConfigProperty API altogether and replace it with injecting config interfaces instead, which should be considerably easier to implement.
  • I'd want to be able to allow external plugins to also examine the configuration interfaces somehow, so I can provide additional functionality (like command-line argument mapping) - or, perhaps that functionality could be added directly to this project.
  • Quarkus parses configuration eagerly, and thus can detect unknown properties within a given namespace, as well as having the ability to populate maps of groups. This seems like a valuable feature to have, but it would mean that we'd also need the ability to tell the config system what subset of keys are required to have been mapped to configuration interfaces (perhaps a glob pattern like quarkus.** would work).
  • Quarkus needs all generated classes to be created no later than class static initialization. So if Quarkus is to use this feature, SmallRye Config would need to be able to do any interface proxying work ahead of time. If the config builder would eagerly do this work, then Quarkus could store partially-prepared config builders in static fields and not have to do any run time generation.
  • It might be useful to produce some tooling for the following purposes:
    • Generate programmatic builder classes corresponding to the config interfaces, which yield a configuration source, for users to include in their APIs in some way
    • Automatic extraction of documentation from configuration interfaces for usage in command-line "usage" messages and also perhaps supplementing JavaDoc or outputting AsciiDoc (similar to what Quarkus does)

Obviously this does not all need to be "day one" functionality.

@dmlloyd
Copy link
Contributor

dmlloyd commented Apr 17, 2020

Another thing to consider is validation. It'd be quite nice to have annotation-driven validation as part of this, which uses the validating converters we provide.

Need to figure out: How lazy should validation be? Sometimes eager validation might be more useful; sometimes lazy might be. Being lazy means storing the configuration value and converting it lazily, which might be fairly inconvenient unless we enhance ConfigValue along the lines that Scott originally proposed way back in eclipse/microprofile-config#43.

@kenfinnigan
Copy link
Contributor Author

These are some awesome ideas, and the idea that interfaces can be used with the builder API aligns with how MP REST Client works as well.

The automatic documentation from annotation is something that @cescoffier has done for Reactive Messaging. It could be something we can extract to a common thing for use by all.

Should all these be separate issues, or leave here for now?

@dmlloyd
Copy link
Contributor

dmlloyd commented Apr 17, 2020

I think it needs to marinate a bit longer since some of the answers to some questions have bearing on what the issues/implementation steps would even be. Let's keep it as one issue for now.

@radcortez
Copy link
Member

We probably need to detail some of these and do it step by step of course.

@radcortez
Copy link
Member

Initial implementation for this is coming in 1.9. We will add additional features as required in separate issues / PRs.

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