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.

Sign up for GitHub

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

Jump to bottom

Support Hocon/Json Configuration Source for MP #4218

Merged
merged 15 commits into from
May 20, 2022
Merged

Support Hocon/Json Configuration Source for MP #4218

merged 15 commits into from
May 20, 2022

Conversation

klustria
Copy link
Member

@klustria klustria commented May 14, 2022 •
edited
Loading

  1. Meta-config is refactored to allow it to be customizable by users for other types.
  2. Hocon/Json is parsed as a config source only via meta-config.

@klustria
Support Hocon/Json Configuration Source for MP
08d13b0 
1. Discovers application.conf as a config source and parses it in hocon format
2. Discovers application.json as a config source and parses it in json format
3. Meta-config Config Source support for new types: hocon and json
4. Config profile support for both hocon and json config sources
@tomas-langer
Copy link
Member

This PR adds HOCON/JSON support as part of the default Helidon MicroProfile Config implementation.
I do not think this is the right approach, as we want to have modular approach as much as possible. We made a (conscious) decision to support YAML, as we use application.yaml in most of our examples and treat it as the expected default in a Helidon application.
We never considered HOCON/JSON to have such a "first class" citizen status in Helidon (SE or MP).

I think we need to refactor our MP metaconfiguration to be based on a service loader, and add a service provider implementation to both YAML and Hocon MP config sources. This provider would add a supported type (such as yaml or hocon) to be used in the MP meta configuration.

This would be a backward compatible change and allow us to add additional MP config sources in the future to be compatible with MP meta configuration.

@klustria
Copy link
Member Author

Thanks for the feedback @tomas-langer. I think I’ve discussed this with you before, but is it ok if I remove the metaconfig change in this PR, and deal with this in a new issue that would take care of the requirements you suggested? Are you ok with the rest of the changes, outside of the metaconfig?

tomas-langer
tomas-langer requested changes May 19, 2022
View reviewed changes
config/config-mp/src/main/java/io/helidon/config/mp/MpMetaConfigUtils.java Show resolved Hide resolved
config/config-mp/src/main/java/io/helidon/config/mp/MpPropertiesMetaConfigProvider.java Outdated Show resolved Hide resolved
config/config-mp/src/main/java/io/helidon/config/mp/MpSystemPropertiesMetaConfigProvider.java Outdated Show resolved Hide resolved
config/config-mp/src/main/java/io/helidon/config/mp/MpPropertiesMetaConfigProvider.java Show resolved Hide resolved
config/config-mp/src/main/java/io/helidon/config/mp/spi/MpMetaConfigProvider.java Outdated Show resolved Hide resolved
...sources/META-INF/native-image/io.helidon.config/helidon-config-hocon-mp/resource-config.json Outdated Show resolved Hide resolved
config/hocon-mp/src/test/java/io/helidon/config/hocon/mp/HoconJsonMpConfigSourceTest.java Outdated Show resolved Hide resolved
config/yaml-mp/src/main/java/module-info.java Show resolved Hide resolved
@klustria klustria mentioned this pull request May 20, 2022
Closed
tomas-langer
tomas-langer previously approved these changes May 20, 2022
View reviewed changes
@klustria klustria merged commit 84451b4 into helidon-io:helidon-2.x May 20, 2022
@klustria klustria mentioned this pull request May 23, 2022
Closed
@klustria klustria mentioned this pull request Jun 2, 2022
Closed
@klustria
Copy link
Member Author

klustria commented Jun 2, 2022

Docs related issues:

  1. Document Hocon/Json MP support  #4251 - Document Hocon/Json MP support
  2. Document MP meta-config extensibility #4253 - Document MP meta-config extensibility

klustria added a commit to klustria/helidon that referenced this pull request Jun 7, 2022
@klustria
Support Hocon/Json Configuration Source for MP (helidon-io#4218)
71337e3 
* Support Hocon/Json Configuration Source for MP

1. Meta-config is refactored to allow it to be customizable by users for other types.
2. Hocon/Json is parsed as a config source only via meta-config.

* Create hocon includer per instance of hocon config source and add justification comment for security bug filtering

* Remove support of application.conf/json hocon/json as default discoverable sources

* Initial commit for Mp config-source provider implementation

* Add MP meta-config provider test for Environment Variables

* Add Yaml MP meta-config provider tests and various resolution to review feedback

* Add helidon-config-yaml-mp as a dependency of helidon-microprofile-cdi

* Declare "uses io.helidon.config.mp.spi.MpMetaConfigProvider" in config-mp’s module-info and set priority of all implementations of MpMetaConfigProvider to 300

* Various updates/cleanups from review feedback

* Add helidon-config-yaml dependency to metrics examples after they were failing because of dependency on yaml-mp
@klustria klustria mentioned this pull request Jun 8, 2022
Merged
klustria added a commit that referenced this pull request Jun 8, 2022
@klustria
Support Hocon/Json Configuration Source for MP (#4347)
bbf31fa 
* Support Hocon/Json Configuration Source for MP (#4218)

* Support Hocon/Json Configuration Source for MP

1. Meta-config is refactored to allow it to be customizable by users for other types.
2. Hocon/Json is parsed as a config source only via meta-config.

* Create hocon includer per instance of hocon config source and add justification comment for security bug filtering

* Remove support of application.conf/json hocon/json as default discoverable sources

* Initial commit for Mp config-source provider implementation

* Add MP meta-config provider test for Environment Variables

* Add Yaml MP meta-config provider tests and various resolution to review feedback

* Add helidon-config-yaml-mp as a dependency of helidon-microprofile-cdi

* Declare "uses io.helidon.config.mp.spi.MpMetaConfigProvider" in config-mp’s module-info and set priority of all implementations of MpMetaConfigProvider to 300

* Various updates/cleanups from review feedback

* Add helidon-config-yaml dependency to metrics examples after they were failing because of dependency on yaml-mp

* Replace javax.inject with jakarta.inject and javax.enterprise with jakarta.enterprise
@klustria klustria mentioned this pull request Jun 9, 2022
Closed
@arjav-desai arjav-desai mentioned this pull request Jun 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomas-langer tomas-langer tomas-langer approved these changes

Assignees

@klustria klustria

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@klustria @tomas-langer