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

MicroProfile Config 3.0 #3644

Merged
merged 6 commits into from
Nov 22, 2021
Merged

Conversation

tomas-langer
Copy link
Member

Implementation of MP Config
Enabled TCK for Config
Fixed all related tests

Resolves #2640

@tomas-langer tomas-langer added the 3.x Issues for 3.x version branch label Nov 15, 2021
@tomas-langer tomas-langer self-assigned this Nov 15, 2021
return findConfigValue(key)
.orElseGet(() -> new ConfigValueImpl(key, null, null, null, 0));
}
return findConfigValue("%" + configProfile + "." + key)
Copy link
Member

@spericas spericas Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the need for all this String concatenation required by the spec for profiles? Seems like not the fastest option

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The specification for profile requires that you first attempt to find the configuration value by the config profile prefix (which implies string concatenation), and then (if not found) you use the original key.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caching is not an option, as MP Config requires the values to be always requested from config source(s)

spericas
spericas previously approved these changes Nov 16, 2021
Copy link
Member

@spericas spericas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some minor comments but otherwise LGTM.

@@ -492,7 +498,12 @@ public Config build() {

// nope, return the result
if (configuredProfile == null) {
LOGGER.fine("Built MP config with no profile");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (LOGGER.isLoggable(Level.FINEST)) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed (also it is fine not finest). If the string is a constant, there is not much overhead doing it this way. The guarding of logs statements should only be used when passing variables to output

@tomas-langer tomas-langer merged commit db35b26 into helidon-io:master Nov 22, 2021
@tomas-langer tomas-langer deleted the 2640-mp-config branch November 22, 2021 15:17
@arjav-desai arjav-desai mentioned this pull request Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Issues for 3.x version branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config 2.0
3 participants