Skip to content

Filter non stork configs and read them as optional values#1204

Merged
aureamunoz merged 2 commits intosmallrye:mainfrom
aureamunoz:optional-properties-issue-1201
Apr 30, 2026
Merged

Filter non stork configs and read them as optional values#1204
aureamunoz merged 2 commits intosmallrye:mainfrom
aureamunoz:optional-properties-issue-1201

Conversation

@aureamunoz
Copy link
Copy Markdown
Collaborator

@aureamunoz aureamunoz commented Apr 21, 2026

Fixes #1201

@aureamunoz aureamunoz requested a review from cescoffier April 21, 2026 07:40
cescoffier
cescoffier previously approved these changes Apr 22, 2026
Copy link
Copy Markdown
Contributor

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

Just 2 minor comments.

@cescoffier
Copy link
Copy Markdown
Contributor

The Spring Boot config provider doesn't have the equivalent filtering / optional value handling:

for (String propertyName : getPropertyNames(environment)) {
    StorkConfigUtils.computeServiceProperty(propertiesByServiceName, propertyName,
            environment.getProperty(propertyName));
}

Should we add it there too?

cescoffier
cescoffier previously approved these changes Apr 24, 2026
@aureamunoz
Copy link
Copy Markdown
Collaborator Author

The Spring Boot config provider doesn't have the equivalent filtering / optional value handling:

for (String propertyName : getPropertyNames(environment)) {
    StorkConfigUtils.computeServiceProperty(propertiesByServiceName, propertyName,
            environment.getProperty(propertyName));
}

Should we add it there too?

Yes, I think so for consistency. I saw it at some point but didn't do. It's done in last commit.

@aureamunoz aureamunoz requested a review from cescoffier April 24, 2026 14:15
cescoffier
cescoffier previously approved these changes Apr 26, 2026
Copy link
Copy Markdown
Contributor

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

See a few minor comments, but nothing is holding the merge.

@aureamunoz aureamunoz force-pushed the optional-properties-issue-1201 branch from 6653966 to 69ff130 Compare April 27, 2026 13:59
@aureamunoz aureamunoz requested a review from cescoffier April 27, 2026 15:19
@aureamunoz
Copy link
Copy Markdown
Collaborator Author

I need your approval again to merge

@aureamunoz aureamunoz merged commit 63aaafa into smallrye:main Apr 30, 2026
7 checks passed
cescoffier added a commit that referenced this pull request May 4, 2026
Refactor improvements

Co-authored-by: Clement Escoffier <clement@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Microprofile Provider: Stork should only call config.getValue() on properties related to Stork

2 participants