-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Update SmallRye Config to 3.8.1 #40225
Conversation
This comment has been minimized.
This comment has been minimized.
bf805bd
to
da60d51
Compare
This comment has been minimized.
This comment has been minimized.
da60d51
to
b94c971
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@zakkak @galderz @yrodiere @maxandersen some good news on the reflection front:
It fails because we are below the threshold :). Now, this is just the methods metrics but it was the one growing significantly every time we migrated a config class to @radcortez needs to adjust the testing a bit on his side of the PR but we should be good for 3.11. |
That's great, well done! |
Great! Just a heads-up: updates to the expected metrics will likely conflict with #40102, so you might want to wait until that's merged before doing all that work. |
To be able to use in core.
We only need to register for reflection config interfaces that are constrained. We cannot be more fine grained at the moment but theoretically we could only register the methods required to go from the root of the config mapping to the (potentially nested) constrained interface. Also make sure config implementors can be properly handled by including them in the indexes considered by HV.
@zakkak just to be clear, it's only improving the methods (I think), so https://github.com/quarkusio/quarkus/pull/40102/files#diff-f740c865e615865a3cf3a4018a7115b9e528d6ad48595ec8e264bb4fc5b2ea83R4 is probably still problematic. |
...ration-tests/smallrye-config/src/main/java/io/quarkus/it/smallrye/config/ServerResource.java
Outdated
Show resolved
Hide resolved
3132581
to
e264d30
Compare
e264d30
to
0a775e3
Compare
Status for workflow
|
Thanks for collaborating with me on this, @radcortez ! |
Hello @radcortez , we have experienced slight change of behavior that I describe here quarkus-qe/quarkus-test-framework#1117. I don't think it is a bug, but the way I read it in past environment variable I could read the situation wrong. I think this is not a bug, but I'd like to check with you that I didn't miss something. Do you read the change of behavior same as I do and is it expected? |
Env variable matching is a bit smarter. Before it was blindly trying to match the expected dotted name. Now, it uses the actual container types to generate the expected name. |
Alright, thanks. |
@ConfigMapping
method reflection registration