There is another similar piece here:

Quite frankly, I have no idea why it was done that way, but I usually prefer to confirm if there is some particular use case that we are not covering with a simple query to the Config system?

//cc @phillip-kruger @michalvavrik

There is another similar piece here:

quarkus/extensions/oidc/deployment/src/main/java/io/quarkus/oidc/deployment/devservices/AbstractDevUIProcessor.java

Lines 93 to 122 in 3c2d7bb

	private static String getProperty(ConfigurationBuildItem configurationBuildItem,
	String propertyKey) {
	// strictly speaking we know 'quarkus.swagger-ui.path' is build time property
	// and 'quarkus.smallrye-graphql.ui.root-path' is build time with runtime fixed,
	// but I wanted to make this bit more robust till we have DEV UI tests
	// that will fail when this get changed in the future, then we can optimize this
	ConfigValue configValue = configurationBuildItem
	.getReadResult()
	.getAllBuildTimeValues()
	.get(propertyKey);
	if (configValue == null || configValue.getValue() == null) {
	configValue = configurationBuildItem
	.getReadResult()
	.getBuildTimeRunTimeValues()
	.get(propertyKey);
	} else {
	return configValue.getValue();
	}
	if (configValue == null || configValue.getValue() == null) {
	configValue = configurationBuildItem
	.getReadResult()
	.getRunTimeDefaultValues()
	.get(propertyKey);
	}
	return configValue != null ? configValue.getValue() : null;
	}

Thanks for spotting this!

Quite frankly, I have no idea why it was done that way, but I usually prefer to confirm if there is some particular use case that we are not covering with a simple query to the Config system?

//cc @phillip-kruger @michalvavrik

By Config system, do you mean using ConfigProvider.getConfig().getValue()?

Anyways, yeah, let's wait for more feedback.

By Config system, do you mean using ConfigProvider.getConfig().getValue()?

Yes, and I'm not sure why the current code is accessing the configuration as is. That suspicious comment requires further clarification.

Quite frankly, I have no idea why it was done that way, but I usually prefer to confirm if there is some particular use case that

we are not covering with a simple query to the Config system?

I always look what others do, it wasn't clear to me for a long time if ConfigProvider.getConfig() inside of a build step is alright. Maybe it was clear to @radcortez, but I looked for it and didn't find it :-). Anyway, this is DEV UI, so changing it is fine. I think I just made mistake, that's all.

I always look what others do, it wasn't clear to me for a long time if ConfigProvider.getConfig() inside of a build step is alright.

Yes, this is ok. We just need to be aware that the

I always look what others do, it wasn't clear to me for a long time if ConfigProvider.getConfig() inside of a build step is alright.

Yes, this is fine. Remeber that the Config instance only has access to build time configuration when in build steps.

Ideally, it should use io.quarkus.swaggerui.deployment.SwaggerUiConfig#path.

@radcortez why did you close this pull request with invalid? I was understanding that my changes about using the ConfigProvider API was ok

Sorry, I've probably hit the wrong button.

@radcortez I've just updated the pull request to also update this: #46670 (comment)

Status for workflow Quarkus CI

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

Failing Jobs

Full information is available in the Build summary check run.
You can consult the Develocity build scans.

Failures

⚙️ JVM Tests - JDK 17 Windows #

- Failing: extensions/websockets-next/deployment 

📦 extensions/websockets-next/deployment

✖ io.quarkus.websockets.next.test.security.HttpUpgradePermitAllAnnotationTest. - History - More details - Source on GitHub

java.lang.RuntimeException: Failed to start quarkus
	at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)
	at io.quarkus.runtime.Application.start(Application.java:101)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.runner.bootstrap.StartupActionImpl.run(StartupActionImpl.java:305)
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:697)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.lang.OutOfMemoryError: Metaspace

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging-kafka/deployment

✖ io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingWorkingAppPropsTestCase.testContinuousTestingScenario3 - History

• io.quarkus.builder.BuildException: Build failure: Build failed due to errors [error]: Build step io.quarkus.apicurio.registry.devservice.DevServicesApicurioRegistryProcessor\#startApicurioRegistryDevService threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image quay.io/apicurio/apicurio-registry-mem:2.4.2.Final at io.quarkus.apicurio.registry.devservice.DevServicesApicurioRegistryProcessor.startApicurioRegistryDevService(DevServicesApicurioRegistryProcessor.java:90) at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:732) at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856) - java.lang.RuntimeException

java.lang.RuntimeException: 
io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.apicurio.registry.devservice.DevServicesApicurioRegistryProcessor#startApicurioRegistryDevService threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image quay.io/apicurio/apicurio-registry-mem:2.4.2.Final
	at io.quarkus.apicurio.registry.devservice.DevServicesApicurioRegistryProcessor.startApicurioRegistryDevService(DevServicesApicurioRegistryProcessor.java:90)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:732)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:255)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)

Hey, @gsmet I've not contributed to Quarkus for a while and wanted to double check if it was ok to merge these changes even when the check "Quarkus CI / JVM Tests - JDK 17 Windows (pull_request)" is failing.
I would say this check is unrelated to these changes.

Hey, @gsmet I've not contributed to Quarkus for a while and wanted to double check if it was ok to merge these changes even when the check "Quarkus CI / JVM Tests - JDK 17 Windows (pull_request)" is failing. I would say this check is unrelated to these changes.

I think I added that test, it should be unrelated. I can't see in Develocity history for OOM happening and I presume OOM can happen to due to what happens before that test is executed, so I'll not debug it for now.