Skip to content

Conversation

@radcortez
Copy link
Member

Following the lengthy discussion at #46915, this is a proposal to introduce a separate API for registering and retrieving values that are only known at runtime and differ from the original configured values.

For compatibility reasons, these values are still exposed in a ConfigSource, but at least internally, we can start dropping the use of Config directly and use RuntimeValues instead (might need a different name :)).

This also includes a rewrite in Vertx HTTP to show how we would use the API with the HTTP ports, and the introduction of a proper (incomplete) API to retrieve the runtime ports instead of relying on Config.

@radcortez
Copy link
Member Author

/cc @dmlloyd @holly-cummins @cescoffier

Copy link
Member

@dmlloyd dmlloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For compatibility reasons, these values are still exposed in a ConfigSource, but at least internally, we can start dropping the use of Config directly and use RuntimeValues instead (might need a different name :)).

As stated in the review comments, I don't think this is necessary. The API can be introduced incrementally without doing a single thing to configuration one way or another.

This also includes a rewrite in Vertx HTTP to show how we would use the API with the HTTP ports, and the introduction of a proper (incomplete) API to retrieve the runtime ports instead of relying on Config.

See my comments about registering port numbers versus socket addresses.

Comment on lines 7 to 31
public class RuntimeValuesConfigSource extends AbstractConfigSource {

public RuntimeValuesConfigSource() {
super("io.quarkus.runtime.RuntimeValuesConfigSource", Integer.MAX_VALUE - 10);
}

@Override
public Set<String> getPropertyNames() {
return Set.of();
}

@Override
public String getValue(String propertyName) {
// TODO - avoid alloc for lookup
Object value = RuntimeValues.config().get(propertyName);
return value != null ? value.toString() : null;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this compatibility layer is completely unnecessary, and probably harmful. I would just introduce the new mechanism, by itself, and then slowly start moving things over to use it.

Changing the code to register things into the map does not break compatibility in any way. Then, changing code to read from the map also doesn't break anything.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but that would mean that we would still have to keep the old mutations of the config in place:
https://github.com/quarkusio/quarkus/pull/51209/files#diff-902af2bee29f3da057d18e68c0836c03bcaeeff5e40e17524801c1261af1bc2fR1355-R1369

The goal here was also to have a way to get rid of this Config mutation. You still have it indirectly via this source, but I think it would be easier to get rid of it once we decide on what to do with the discovery mechanism.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we should get rid of the config mutations, however replacing one interim solution with another isn't worth it IMO. They don't have to be removed immediately as long as there is a plan to remove them eventually.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a first step for eventually:

  • We are going to touch on the same areas where the config mutations are happening to register the runtime values.
  • We don't really have "an official" way to mutate the config, so there are plenty of solutions and workarounds around the code to achieve. This would be a single indirect way.
  • This would give us a possible list of configuration names that users refer to (or we use internally), expecting certain values. We know some, but we don't know all. This could allow us to build such a list.
  • With that list, we could start issuing warnings saying that the name shouldn't be accessed through Config and point to the proper API.
  • It would be easier to remove once we have our complete solution without having to revisit all the places around the code again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. If you feel strongly about it, then I'm with you. But we do need to ensure the thread-safety of what's going on here.

Comment on lines +9 to +14
public static final RuntimeKey<Integer> HTTP_PORT = RuntimeKey.intKey("quarkus.http.port");
public static final RuntimeKey<Integer> HTTP_TEST_PORT = RuntimeKey.intKey("quarkus.http.test-port");
public static final RuntimeKey<Integer> HTTPS_PORT = RuntimeKey.intKey("quarkus.http.ssl-port");
public static final RuntimeKey<Integer> HTTPS_TEST_PORT = RuntimeKey.intKey("quarkus.http.test-ssl-port");
public static final RuntimeKey<Integer> MANAGEMENT_PORT = RuntimeKey.intKey("quarkus.management.port");
public static final RuntimeKey<Integer> MANAGEMENT_TEST_PORT = RuntimeKey.intKey("quarkus.management.test-port");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't we want to store the actual socket address(es) rather than just the port number?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is something we can add. This was just to showcase it with the port number.

@quarkus-bot

This comment has been minimized.

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 25, 2025

Status for workflow Quarkus CI

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

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
Native Tests - gRPC Build Failures Logs Raw logs 🔍
✔️ JVM Integration Tests - JDK 17 Logs Raw logs 🚧
✔️ JVM Integration Tests - JDK 17 Windows Logs Raw logs 🚧
JVM Integration Tests - JDK 21 Build Failures Logs Raw logs 🚧
✔️ JVM Integration Tests - JDK 21 Semeru Logs Raw logs 🚧
✔️ JVM Integration Tests - JDK 25 Logs Raw logs 🚧

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

Failures

⚙️ Native Tests - gRPC #

- Failing: integration-tests/grpc-test-random-port integration-tests/grpc-tls integration-tests/grpc-tls-p12 

📦 integration-tests/grpc-test-random-port

io.quarkus.grpc.examples.hello.RandomPortVertxServerPlainIT.testWithNative - History - More details - Source on GitHub

java.lang.RuntimeException: java.lang.IllegalArgumentException: Key already registered quarkus.http.port with value 8081
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.throwBootFailureException(QuarkusIntegrationTestExtension.java:345)
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.beforeEach(QuarkusIntegrationTestExtension.java:113)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.lang.IllegalArgumentException: Key already registered quarkus.http.port with value 8081
	at io.quarkus.runtime.RuntimeValues.register(RuntimeValues.java:31)
	at io.quarkus.test.common.LauncherUtil.registerRuntimeValues(LauncherUtil.java:219)

📦 integration-tests/grpc-tls

io.quarkus.grpc.examples.hello.VertxHelloWorldTlsEndpointIT.testHelloWorldServiceUsingBlockingStub - History - More details - Source on GitHub

java.lang.RuntimeException: java.lang.IllegalArgumentException: Key already registered quarkus.http.port with value 8081
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.throwBootFailureException(QuarkusIntegrationTestExtension.java:345)
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.beforeEach(QuarkusIntegrationTestExtension.java:113)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.lang.IllegalArgumentException: Key already registered quarkus.http.port with value 8081
	at io.quarkus.runtime.RuntimeValues.register(RuntimeValues.java:31)
	at io.quarkus.test.common.LauncherUtil.registerRuntimeValues(LauncherUtil.java:219)

📦 integration-tests/grpc-tls-p12

io.quarkus.grpc.examples.hello.HelloWorldTlsEndpointIT.testHelloWorldServiceUsingBlockingStub - History - More details - Source on GitHub

java.lang.RuntimeException: java.lang.RuntimeException: Unable to successfully launch process '3250'. Exit code is: '1'.
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.throwBootFailureException(QuarkusIntegrationTestExtension.java:345)
	at io.quarkus.test.junit.QuarkusIntegrationTestExtension.beforeEach(QuarkusIntegrationTestExtension.java:113)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.lang.RuntimeException: Unable to successfully launch process '3250'. Exit code is: '1'.
	at io.quarkus.test.common.LauncherUtil.ensureProcessIsAlive(LauncherUtil.java:110)
	at io.quarkus.test.common.LauncherUtil.waitForCapturedListeningData(LauncherUtil.java:73)

⚙️ JVM Integration Tests - JDK 21 #

- Failing: integration-tests/spring-web 

📦 integration-tests/spring-web

io.quarkus.it.spring.web.SpringSchedulerTest. - History - More details - Source on GitHub

org.junit.jupiter.engine.execution.ConditionEvaluationException: Failed to evaluate condition [io.quarkus.test.junit.QuarkusTestExtension]: Internal error: Test class was loaded with an unexpected classloader (QuarkusClassLoader:Quarkus Base Runtime ClassLoader: TEST for JUnitQuarkusTest-no-profile (QuarkusTest)@b87ea8b) or the thread context classloader (jdk.internal.loader.ClassLoaders$AppClassLoader@c387f44) was incorrect.
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1685)
	at java.base/java.util.stream.Reference...

io.quarkus.it.spring.web.SpringSchedulerTest.testCount line 22 - History - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:31)
	at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:183)
	at io.quarkus.it.spring.web.SpringSchedulerTest.testCount(SpringSchedulerTest.java:22)

Flaky tests - Develocity

⚙️ MicroProfile TCKs Tests

📦 tcks/microprofile-lra

org.eclipse.microprofile.lra.tck.TckRecoveryTests.testCancelWhenParticipantIsUnavailable - History

  • Expecting the metric Compensated callback was called Expected: a value equal to or greater than <1> but: <0> was less than <1> - java.lang.AssertionError
java.lang.AssertionError: 
Expecting the metric Compensated callback was called
Expected: a value equal to or greater than <1>
     but: <0> was less than <1>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.eclipse.microprofile.lra.tck.TckRecoveryTests.assertMetricCallbackCalled(TckRecoveryTests.java:210)
	at org.eclipse.microprofile.lra.tck.TckRecoveryTests.testCancelWhenParticipantIsUnavailable(TckRecoveryTests.java:195)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants