-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Dev UI/Swagger: Support expansion of properties in path #46670
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
There is another similar piece here: Lines 93 to 122 in 3c2d7bb
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 |
Thanks for spotting this!
By Anyways, yeah, let's wait for more feedback. |
Yes, and I'm not sure why the current code is accessing the configuration as is. That suspicious comment requires further clarification. |
I always look what others do, it wasn't clear to me for a long time if |
Yes, this is ok. We just need to be aware that the |
Yes, this is fine. Remeber that the Ideally, it should use |
@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. |
This comment has been minimized.
This comment has been minimized.
@radcortez I've just updated the pull request to also update this: #46670 (comment) |
Status for workflow
|
Status | Name | Step | Failures | Logs | Raw logs | Build scan |
---|---|---|---|---|---|---|
✔️ | JVM Tests - JDK 17 | Logs | Raw logs | 🔍 | ||
✔️ | JVM Tests - JDK 21 | Logs | Raw logs | 🔍 | ||
✖ | JVM Tests - JDK 17 Windows | Build |
Failures | Logs | Raw logs | 🔍 |
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 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. |
When configuring the swagger UI path as:
Note that you can use any property other than
quarkus.application.name
.This fails with:
This issue was introduced as part of the changes in #43950.
One solution is to use the ConfigProvider API to resolve the property since this already expand all the properties. @radcortez can you confirm if this is ok?
Testing these changes, I can now see that the path is correct:
This is using a Quarkus example named "resteasy".
Fixes #46662