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

Quarkus 2.12.0.CR1 - Map style config property resolution does not work in native mode #27353

Closed
jamesnetherton opened this issue Aug 18, 2022 · 6 comments · Fixed by #27660
Closed
Assignees
Labels
Milestone

Comments

@jamesnetherton
Copy link
Contributor

jamesnetherton commented Aug 18, 2022

Describe the bug

In Camel Quarkus it's possible to have some configuration like the following in application.properties:

camel.threadpool.config[customPool].id = customPool
camel.threadpool.config[customPool].pool-size = 1
camel.threadpool.config[customPool].rejectedPolicy = Abort

Where the config[customPool] part denotes a map key that Camel can resolve internally. This all works fine in JVM mode. Since 2.12.0.CR1, resolution of such properties no longer seems to work in native mode. The config keys seem to be present, but their associated values are not resolvable.

Expected behavior

The properties are resolvable as expected in native mode.

Actual behavior

See comments in the main description.

How to Reproduce?

There's a reproducer project here:

https://github.com/jamesnetherton/smallrye-config-demo

See instructions in the README for how to run. Tests fail with 2.12.0.CR1, they pass with 2.11.2.Final.

@gsmet
Copy link
Member

gsmet commented Aug 18, 2022

@radcortez could you have a look at this one? It looks like a blocker for 2.12.

@gsmet gsmet added this to the 2.12.0.Final milestone Aug 18, 2022
@geoand
Copy link
Contributor

geoand commented Aug 23, 2022

This is almost certainly caused by #26802.

The reason it works in JVM mode is because in that case the property read directly from the JAR file.
However in native mode, the property is attempted to be read by DefaultsConfigSource and it's superclass (KeyMapBackedConfigSource which I don't really understand as it's complex and I've never looked at it before) does not handle [ and ] correctly.

This specific issue would be fixed if we used MapBackedConfigSource as the superclass instead, but I don't know what other implications that might have, so I think it's best we wait for @radcortez on this.

EDIT: I just saw that what I explained above is pretty much also mentioned in #27231 (comment)

@gsmet
Copy link
Member

gsmet commented Aug 23, 2022

So question: is the cure worse than the disease? My understanding is that any map based config won't work in native from now on? It seems like something problematic and I wonder if we should revert this particular patch until @radcortez can revisit it.

WDYT?

@geoand
Copy link
Contributor

geoand commented Aug 23, 2022

Only bracket based keys are problematic (and then only in native mode). The Quarkus Config maps using . still work properly.

@gsmet
Copy link
Member

gsmet commented Aug 23, 2022

After discussion with @geoand , it only affects using brackets which is not what we recommend (we recommend the dot notation even for maps). So we can live with it for .0.Final and we will discuss it when @radcortez is back.

@gsmet gsmet removed this from the 2.12.0.Final milestone Aug 24, 2022
@radcortez
Copy link
Member

This requires a new release of SR Config with smallrye/smallrye-config#805.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants