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

Configuration value location #59

Closed
dmlloyd opened this issue Dec 6, 2018 · 9 comments · Fixed by #262
Closed

Configuration value location #59

dmlloyd opened this issue Dec 6, 2018 · 9 comments · Fixed by #262
Projects
Milestone

Comments

@dmlloyd
Copy link
Contributor

dmlloyd commented Dec 6, 2018

It would be nice to be able to track and report the source of a bad (invalid) configuration value. This might not be apparent, depending on how the configuration value was set.

Such an API would provide a method to get a location given a configuration key (this might fit best on ConfigSource if there is a way to identify which ConfigSource a key came from). The location would contain a "source description" which could be a file name, URL, or description such as "system property" or "environment variable". It would also contain an optional line number, which could be -1 if the line number isn't known or doesn't make sense for this location.

@jmesnil
Copy link
Contributor

jmesnil commented Dec 7, 2018

The Config API has all the primitives to provide this feature:

String configName = ...
Config config = ...
For each configSource in config { // they are returned according to their ordinals, the first value found is the one that is returned by config.getValue(...)
  if configSource.hasKey(configName) {
    print(configSource.getName()) // the name will display the type and origin (eg file path) of the config source
    print(configSource.getOrdinal())
    print(configSource.getProperties().get(configName) // <- value for this config source
  }
}

I have a dev branch for WildFly that is exposing this feature as a management operation: jmesnil/wildfly-microprofile-config@17d5abe#diff-a92756bd13114c03b7196011c6bf80baR108

I don't think there is anything to add to the API for this RFE.
However we still need a place to put this code.
It'd be a waste to expect every Config user to have to write this same code over and over.

We could in a first step add a static class to smallrye-config that provides this feature (and let the user specify what to do with the found config sources?).
In a second step, we can propose to add this help class to the microProfile-config API.

@jmesnil
Copy link
Contributor

jmesnil commented Dec 7, 2018

@dmlloyd something like that should meet your requirements? https://gist.github.com/jmesnil/946ca36058fc3b0aa14755986798c14c

@dmlloyd
Copy link
Contributor Author

dmlloyd commented Dec 7, 2018

That's the first stage. The second stage would be a method on ConfigSource to query the source location of the given key. The location object should contain a source name and a line number. It could also contain a link to a "parent" location from which that item was included. Ideally it would have a useful toString() that summarizes the "include stack" in this case, much like an exception stack trace.

In general I'm skeptical of TriConsumer though, looks like a code smell ;)

@smoyer64
Copy link
Contributor

smoyer64 commented Dec 7, 2018

@dmlloyd We're using smallrye-config with weld-se-core in a few projects and I've been working towards making the applications reporting better on a failed start-up. The first step was adopted into smallrye-config in 9258e1d which reports configuration failures into the DeploymentException individually. The idea is that CDI containers can "reformat" these exceptions into detailed messages in our enterprise logging system.

The second step is a work-in-progress (which I hope to get back to soon) in weld-se-core itself and is described in weld/core#1838 and the associated Jira issue. This exception handler will then decompose the DeploymentException immediately before the JRE shutdown hooks are called.

@jmesnil
Copy link
Contributor

jmesnil commented Dec 8, 2018

ah, I missed the bits about identifying the "origin/location" from the config property inside the config source. Yes, this part will need to modify the API and find an abstraction that makes sense for a ConfigSource that can be backed by many different things that are not files (Properties, ConfigMap`, DB).

TriConsumer would not cut it for an actual implementation (it's just that I suck at naming things :)

@smoyer64
Copy link
Contributor

smoyer64 commented Dec 8, 2018

it's just that I suck at naming things

Naming is the hardest problem in programming ;) ... you can always take the easy way out and use the Greek or Latin equivalent of TriConsumer.

@dmlloyd
Copy link
Contributor Author

dmlloyd commented Dec 13, 2018

For the non-files bit: that's why I said configuration source (lowercase). It could be a simple String that can be set to something that makes sense by the provider, for example:

  • file:///home/user/app/my-config.yaml
  • configuration.xml
  • System property
  • Environment property
  • Database at mydb.foo.com:12345

@kenfinnigan kenfinnigan added this to To do in Planning Jun 21, 2019
@radcortez
Copy link
Member

This is related with some of the prototype work discussed here to support Config Lifecycle Events and ahead of time declaration:
eclipse/microprofile-config#526

radcortez referenced this issue in radcortez/smallrye-config Mar 13, 2020
…rs and trace the origin of a configuration key.
radcortez referenced this issue in radcortez/smallrye-config Mar 16, 2020
…rceptors and trace the origin of a configuration key.
radcortez referenced this issue in radcortez/smallrye-config Mar 24, 2020
…rceptors and trace the origin of a configuration key.
radcortez referenced this issue in radcortez/smallrye-config Mar 24, 2020
…rceptors and trace the origin of a configuration key.
radcortez referenced this issue in radcortez/smallrye-config Mar 24, 2020
radcortez referenced this issue in radcortez/smallrye-config Mar 24, 2020
radcortez referenced this issue in radcortez/smallrye-config Mar 24, 2020
radcortez referenced this issue in radcortez/smallrye-config Mar 24, 2020
radcortez referenced this issue in radcortez/smallrye-config Mar 25, 2020
radcortez referenced this issue in radcortez/smallrye-config Mar 25, 2020
radcortez referenced this issue in radcortez/smallrye-config Mar 25, 2020
radcortez referenced this issue in radcortez/smallrye-config Mar 25, 2020
radcortez referenced this issue in radcortez/smallrye-config Mar 25, 2020
radcortez referenced this issue in radcortez/smallrye-config Mar 30, 2020
@radcortez
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Planning
  
To do
Development

Successfully merging a pull request may close this issue.

4 participants