Do not discover properties resource in the classpath to avoid missing resources registration in native mode #44910

radcortez · 2024-12-03T21:34:11Z

Disable automatic discovery of configuration resources in native mode.

While it has always been enabled, it didn't have any effect because we never registered the resources in the native image. The configuration still worked, because we recorded the configuration during build time.

I did try to disable it as well for JVM mode, but it is a bit more tricky:

Our testing API can add additional classpath resources after we build Quarkus, which wouldn't be caught with this.
Debugging would be harder, because it would lose the the origin source from where the configuration is coming from.
Flattening has a corner case issue described in Register default configuration smallrye configuration files as resources #44652 (comment). This was never really an issue, but there is a test in the TCK that fails because of this.

Ideally, both JVM and native mode should be the same, but at the moment they're not. While this doesn't solve that particular issue, it should at least fix the missing registration warnings / errors without changing the current behaviour.

Fixes Enhance SmallRyeConfig handling #41994

Alternate proposal to #42140.

geoand · 2024-12-04T09:20:48Z

CI failures seem related

radcortez · 2024-12-04T10:46:07Z

CI failures seem related

Yes

radcortez · 2024-12-04T10:46:26Z

I was expecting more actually :)

github-actions · 2024-12-04T14:11:41Z

🎊 PR Preview 43f9421 has been successfully built and deployed to https://quarkus-pr-main-44910-preview.surge.sh/version/main/guides/

Images of blog posts older than 3 months are not available.
Newsletters older than 3 months are not available.

zakkak · 2025-01-10T10:15:42Z

@radcortez what's the status of this PR, is it ready for review and merging?

radcortez · 2025-01-10T11:25:02Z

Yes, if you are happy with it :)

zakkak

LGTM, but I have a concern about this being a breaking change, see comments.

zakkak · 2025-01-10T13:50:57Z

core/runtime/src/main/java/io/quarkus/runtime/configuration/NativeConfigBuilder.java

+ * configuration will still be available. If the users change the ordinals of the sources in runtime, it may
+ * cause unexpected values to be returned by the config, but this has always been the case. The classpath configuration
+ * resources were never registered in the native image.


Is this true?

In oracle/graal#10264 (reply in thread) we describe a scenario were using the classpath would work fine (assuming the user registers the resource for reflection and sets the ordinals accordingly).

With our stuff yes, we never registered the resources. The configuration is just included in a class file and we discard the resources, which may cause the issue described in #44652 (comment), but this has been an issue since day one and until now we never got any complaints about it.

zakkak · 2025-01-10T13:52:05Z

core/runtime/src/main/java/io/quarkus/runtime/configuration/Substitutions.java

+    static final class Target_AbstractLocationConfigSourceLoader {
+        @Substitute
+        protected List<ConfigSource> tryClassPath(final URI uri, final int ordinal, final ClassLoader classLoader) {
+            return Collections.emptyList();


If what my previous comment is right, this is a breaking (for quite a corner case I admit) change, no?

Yes, this could be a breaking change if a user has a custom source with a custom resource. If they are registering the resource manually they would need to process the file and generate some sort of representation (like us) for runtime.

Unfortunately, I don't see any good alternative. Having to forcely register all the resources is going to throw away all of our optimizations. If we leave it as is, you get the error that we are trying to prevent. (Well I guess we could add a try / catch there instead).

I still think it doesn't make sense to apply that validation to an API like ClassLoader#getResources. That API never fails in JVM mode, no matter what you are looking for. Granted that you can obtain different results from JVM to native if the registrations are not done properly, but the native image shouldn't try to outsmart the library or integrator by changing API behaviours.

Unfortunately, I don't see any good alternative.

I am personally fine with this, since it's a corner case. I am not even sure we need to document this kind of "breakage".

@radcortez I went on with adding a try/catch. Please have a look.

… resources registration in native mode

Works around issue discussed in oracle/graal#10264 (reply in thread)

quarkus-bot · 2025-01-14T10:39:39Z

Status for workflow `Quarkus Documentation CI`

This is the status report for running Quarkus Documentation CI on commit 8733462.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

quarkus-bot · 2025-01-14T16:25:00Z

Status for workflow `Quarkus CI`

This is the status report for running Quarkus CI on commit 8733462.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/grpc-hibernate

✖ com.example.grpc.hibernate.BlockingRawTest.shouldAdd - History

Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at com.example.grpc.hibernate.BlockingRawTestBase.shouldAdd(BlockingRawTestBase.java:59)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

quarkus-bot bot added area/config area/core area/testing labels Dec 3, 2024

radcortez mentioned this pull request Dec 3, 2024

Limit external configuration sources parsing in native-mode #42140

Closed

This comment has been minimized.

Sign in to view

quarkus-bot bot added the triage/flaky-test label Dec 4, 2024

radcortez force-pushed the disable-config-properties-scan branch from 74e2236 to 8cbb860 Compare December 4, 2024 13:33

radcortez changed the title ~~Do not discover properties resource in the classpath since they are already recorded during build time~~ Do not discover properties resource in the classpath to avoid missing resources registration in native mode Dec 4, 2024

This comment has been minimized.

Sign in to view

radcortez force-pushed the disable-config-properties-scan branch from 8cbb860 to 784a1fe Compare December 11, 2024 18:47

This comment has been minimized.

Sign in to view

zakkak mentioned this pull request Dec 20, 2024

Enable ThrowMissingRegistrationErrors with GraalVM >= 23.1 #36378

Draft

zakkak approved these changes Jan 10, 2025

View reviewed changes

radcortez and others added 2 commits January 14, 2025 11:37

Do not discover properties resource in the classpath to avoid missing…

f082100

… resources registration in native mode

Catch MissingReflectionRegistrationError in loadConfigSources

8733462

Works around issue discussed in oracle/graal#10264 (reply in thread)

zakkak force-pushed the disable-config-properties-scan branch from 784a1fe to 8733462 Compare January 14, 2025 09:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not discover properties resource in the classpath to avoid missing resources registration in native mode #44910

Do not discover properties resource in the classpath to avoid missing resources registration in native mode #44910

radcortez commented Dec 3, 2024 •

edited

Loading

This comment has been minimized.

geoand commented Dec 4, 2024

radcortez commented Dec 4, 2024

radcortez commented Dec 4, 2024

This comment has been minimized.

github-actions bot commented Dec 4, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

zakkak commented Jan 10, 2025

radcortez commented Jan 10, 2025

zakkak left a comment

zakkak Jan 10, 2025

radcortez Jan 10, 2025

zakkak Jan 10, 2025

radcortez Jan 10, 2025

zakkak Jan 13, 2025

zakkak Jan 14, 2025

quarkus-bot bot commented Jan 14, 2025

quarkus-bot bot commented Jan 14, 2025 •

edited by github-actions bot

Loading

Do not discover properties resource in the classpath to avoid missing resources registration in native mode #44910

Are you sure you want to change the base?

Do not discover properties resource in the classpath to avoid missing resources registration in native mode #44910

Conversation

radcortez commented Dec 3, 2024 • edited Loading

This comment has been minimized.

geoand commented Dec 4, 2024

radcortez commented Dec 4, 2024

radcortez commented Dec 4, 2024

This comment has been minimized.

github-actions bot commented Dec 4, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

zakkak commented Jan 10, 2025

radcortez commented Jan 10, 2025

zakkak left a comment

Choose a reason for hiding this comment

zakkak Jan 10, 2025

Choose a reason for hiding this comment

radcortez Jan 10, 2025

Choose a reason for hiding this comment

zakkak Jan 10, 2025

Choose a reason for hiding this comment

radcortez Jan 10, 2025

Choose a reason for hiding this comment

zakkak Jan 13, 2025

Choose a reason for hiding this comment

zakkak Jan 14, 2025

Choose a reason for hiding this comment

quarkus-bot bot commented Jan 14, 2025

Status for workflow Quarkus Documentation CI

quarkus-bot bot commented Jan 14, 2025 • edited by github-actions bot Loading

Status for workflow Quarkus CI

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/grpc-hibernate

radcortez commented Dec 3, 2024 •

edited

Loading

github-actions bot commented Dec 4, 2024 •

edited

Loading

Status for workflow `Quarkus Documentation CI`

quarkus-bot bot commented Jan 14, 2025 •

edited by github-actions bot

Loading

Status for workflow `Quarkus CI`