-
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
Only record config properties coming from ConfigSources #20590
Conversation
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 868d740
Failures⚙️ Initial JDK 11 Build #- Failing: integration-tests/devmode
📦 integration-tests/devmode✖ |
@radcortez can you rebase the change? |
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 868d740
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 #- Failing: core/test-extension/deployment extensions/resteasy-reactive/rest-client-reactive/deployment extensions/smallrye-graphql/deployment and 1 more
! Skipped: docs extensions/oidc-client-reactive-filter/deployment extensions/resteasy-reactive/rest-client-reactive-jackson/deployment and 11 more 📦 core/test-extension/deployment✖
📦 extensions/resteasy-reactive/rest-client-reactive/deployment✖
📦 extensions/smallrye-graphql/deployment✖
✖
📦 integration-tests/oidc✖
⚙️ JVM Tests - JDK 11 Windows #- Failing: core/test-extension/deployment extensions/resteasy-reactive/rest-client-reactive/deployment extensions/smallrye-graphql/deployment
! Skipped: docs extensions/oidc-client-reactive-filter/deployment extensions/resteasy-reactive/rest-client-reactive-jackson/deployment and 11 more 📦 core/test-extension/deployment✖
📦 extensions/resteasy-reactive/rest-client-reactive/deployment✖
📦 extensions/smallrye-graphql/deployment✖
✖
⚙️ JVM Tests - JDK 17 #- Failing: core/test-extension/deployment extensions/hibernate-orm/deployment extensions/resteasy-reactive/rest-client-reactive/deployment and 1 more
! Skipped: docs extensions/hibernate-envers/deployment extensions/hibernate-reactive/deployment and 91 more 📦 core/test-extension/deployment✖
📦 extensions/hibernate-orm/deployment✖
📦 extensions/resteasy-reactive/rest-client-reactive/deployment✖
📦 integration-tests/oidc✖
⚙️ MicroProfile TCKs Tests #- Failing: tcks/microprofile-opentracing/base
! Skipped: tcks/microprofile-opentracing/rest-client 📦 tcks/microprofile-opentracing/base✖
✖
✖
✖
✖
✖
✖
✖
✖
✖
✖
✖
✖
✖
✖
✖
✖
⚙️ Native Tests - Misc3 #- Failing: integration-tests/smallrye-graphql
📦 integration-tests/smallrye-graphql✖
|
hmm... there's quite a lot of failures that I haven't seen before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the failures need to be investigated before merging
Yes, maybe the change broke something else. Let me check. |
868d740
to
bef3a20
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building bef3a20
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 #- Failing: extensions/smallrye-graphql/deployment integration-tests/oidc
! Skipped: docs extensions/smallrye-graphql-client/deployment integration-tests/hibernate-orm-graphql-panache and 2 more 📦 extensions/smallrye-graphql/deployment✖
✖
📦 integration-tests/oidc✖
⚙️ JVM Tests - JDK 11 Windows #- Failing: extensions/smallrye-graphql/deployment
! Skipped: docs extensions/smallrye-graphql-client/deployment integration-tests/hibernate-orm-graphql-panache and 2 more 📦 extensions/smallrye-graphql/deployment✖
✖
⚙️ JVM Tests - JDK 17 #- Failing: extensions/smallrye-graphql/deployment extensions/smallrye-reactive-messaging-kafka/deployment integration-tests/oidc
! Skipped: docs extensions/smallrye-graphql-client/deployment integration-tests/hibernate-orm-graphql-panache and 6 more 📦 extensions/smallrye-graphql/deployment✖
✖
📦 extensions/smallrye-reactive-messaging-kafka/deployment✖
📦 integration-tests/oidc✖
⚙️ Native Tests - Misc3 #- Failing: integration-tests/smallrye-graphql
📦 integration-tests/smallrye-graphql✖
|
This is going to require #20494. |
@radcortez so with https://github.com/quarkusio/quarkus/pull/20602/files the tests are passing? |
Not exactly. The change that I did here uncovered an issue with GraphQL, that is related to #20494. GraphQL is also using an interceptor to relocate some properties, but only for build time (when some of the properties are runtime). Because we removed here the recording of interceptor properties, these keys are not recorded, but they are also not reread on runtime because the GraphQL configuration is set up to be build-time only. |
bef3a20
to
a65d922
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building a65d922
Full information is available in the Build summary check run. Failures⚙️ Native Tests - Data1 #- Failing: integration-tests/hibernate-orm-tenancy/connection-resolver-legacy-qualifiers
📦 integration-tests/hibernate-orm-tenancy/connection-resolver-legacy-qualifiers✖
✖
✖
✖
✖
|
a65d922
to
a44f19a
Compare
I think it should be backported to 2.4, @radcortez is there any other related PR that has to be backported too? |
+1 on backport |
Reasoning for triage: Without this change, subsystems that use MP Config interceptors (like the FallbackConfigSourceInterceptor) can see wrong configuration values under some circumstances. Specifically some values registered during build time are not correctly overwritten by runtime config. |
Yes. It requires #20494. |
it seems #20494 is already in 2.3.x so this one is good to backport |
No description provided.