-
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
Remove Dev UI and other Dev Mode classes from prod #44250
Conversation
/cc @gsmet (hibernate-orm,hibernate-search), @marko-bekhta (hibernate-search), @yrodiere (hibernate-orm,hibernate-search) |
bee38b0
to
b5a19d6
Compare
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
b5a19d6
to
c8a1f44
Compare
Signed-off-by: Phillip Kruger <[email protected]>
c8a1f44
to
f37ce27
Compare
Status for workflow
|
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.
This for me seem like a natural progression :)
I never liked that devui 2 added content to product jars that is not needed in production.
Since this is using existing mechanics I think its a good move.
I would like @aloubyansky or @dmlloyd to review it.
@@ -887,6 +887,19 @@ JsonRPCProvidersBuildItem createJsonRPCServiceForCache() {// <2> | |||
|
|||
https://github.com/quarkusio/quarkus/blob/main/extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/devui/CacheDevUiProcessor.java[Example code] | |||
|
|||
Also cleanup this in Prod mode, so when the app build in prod mode, we remove all dev-ui related runtime classes: |
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.
this pops up kinda in the middle in jsonrpc explanation. Maybe have a more explicit section/title like "Remove devUI from Production" ?
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.
It only really apply to Dev UI that has jsonrpc services. Many extensions does not have any. Any screen (js) and any build time data or runtime against the deployment cp is already not included in the prod build, it's just when you do the jsonrpc service, hence its added here
---- | ||
<1> Always only do this in Normal (Prod) Mode | ||
<2> Produce or return a `DevModeCleanupBuildItem` | ||
<3> Define the class or classes in your runtime module that will be removed. You can also clean the whole package of that class in case you used more than just the JSON-RPC Service |
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.
show how to clean the whole package as otherwise one is left guessing :)
Oh, this doesn't look like a proper way of fixing the issue. Ideally, these classes/dependencies should be properly isolated. Was there a discussion about this? |
Status for workflow
|
Status | Name | Step | Failures | Logs | Raw logs | Build scan |
---|---|---|---|---|---|---|
✖ | JVM Tests - JDK 17 | Build |
Failures | Logs | Raw logs | 🔍 |
✖ | JVM Tests - JDK 21 | Build |
Failures | Logs | Raw logs | 🔍 |
✖ | JVM Tests - JDK 17 Windows | Build |
Failures | Logs | Raw logs | 🔍 |
✖ | Maven Tests - JDK 17 | Build |
Failures | Logs | Raw logs | 🔍 |
✖ | Maven Tests - JDK 17 Windows | Build |
Failures | Logs | Raw logs | 🔍 |
✖ | Gradle Tests - JDK 17 | Build |
Failures | Logs | Raw logs | 🔍 |
✖ | Gradle 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 #
- Failing: integration-tests/kotlin integration-tests/scala
📦 integration-tests/kotlin
✖ io.quarkus.kotlin.maven.it.KotlinRemoteDevModeIT.testThatTheApplicationIsReloadedOnKotlinChange
line 23
- History - More details - Source on GitHub
📦 integration-tests/scala
✖ io.quarkus.scala.maven.it.ScalaRemoteDevModeIT.testThatTheApplicationIsReloadedOnScalaChange
line 24
- History - More details - Source on GitHub
⚙️ JVM Tests - JDK 21 #
- Failing: integration-tests/kotlin
📦 integration-tests/kotlin
✖ io.quarkus.kotlin.maven.it.KotlinRemoteDevModeIT.testThatTheApplicationIsReloadedOnKotlinChange
line 23
- History - More details - Source on GitHub
⚙️ JVM Tests - JDK 17 Windows #
- Failing: integration-tests/kotlin integration-tests/scala
📦 integration-tests/kotlin
✖ io.quarkus.kotlin.maven.it.KotlinRemoteDevModeIT.testThatTheApplicationIsReloadedOnKotlinChange
line 23
- History - More details - Source on GitHub
📦 integration-tests/scala
✖ io.quarkus.scala.maven.it.ScalaRemoteDevModeIT.testThatTheApplicationIsReloadedOnScalaChange
line 24
- History - More details - Source on GitHub
⚙️ Maven Tests - JDK 17 #
- Failing: integration-tests/maven
📦 integration-tests/maven
✖ io.quarkus.maven.it.JarRunnerIT.reaugmentationWithRemovedArtifacts
line 315
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.reaugmentationWithRemovedArtifacts
line 315
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.reaugmentationWithRemovedArtifactsUsingSystemProperties
line 416
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.reaugmentationWithRemovedArtifactsUsingSystemProperties
line 416
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.testThatMutableFastJarWorks
line 198
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.testThatMutableFastJarWorks
line 198
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.testThatMutableFastJarWorksProvidersDirOutsideOutputDir
line 203
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.testThatMutableFastJarWorksProvidersDirOutsideOutputDir
line 203
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatApplicationRecoversCompilationIssue
line 166
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatApplicationRecoversCompilationIssue
line 166
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatNewBeanAreDiscovered
line 204
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatNewBeanAreDiscovered
line 204
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnConfigChange
line 100
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnConfigChange
line 100
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnJavaChange
line 33
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnJavaChange
line 33
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnNewResource
line 67
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnNewResource
line 67
- History - More details - Source on GitHub
⚙️ Maven Tests - JDK 17 Windows #
- Failing: integration-tests/maven
📦 integration-tests/maven
✖ io.quarkus.maven.it.JarRunnerIT.reaugmentationWithRemovedArtifacts
line 315
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.reaugmentationWithRemovedArtifacts
line 315
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.reaugmentationWithRemovedArtifactsUsingSystemProperties
line 416
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.reaugmentationWithRemovedArtifactsUsingSystemProperties
line 416
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.testThatMutableFastJarWorks
line 198
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.testThatMutableFastJarWorks
line 198
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.testThatMutableFastJarWorksProvidersDirOutsideOutputDir
line 203
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.JarRunnerIT.testThatMutableFastJarWorksProvidersDirOutsideOutputDir
line 203
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatApplicationRecoversCompilationIssue
line 166
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatApplicationRecoversCompilationIssue
line 166
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatNewBeanAreDiscovered
line 204
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatNewBeanAreDiscovered
line 204
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnConfigChange
line 100
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnConfigChange
line 100
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnJavaChange
line 33
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnJavaChange
line 33
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnNewResource
line 67
- History - More details - Source on GitHub
✖ io.quarkus.maven.it.RemoteDevMojoIT.testThatTheApplicationIsReloadedOnNewResource
line 67
- History - More details - Source on GitHub
⚙️ Gradle Tests - JDK 17 #
- Failing: integration-tests/gradle
📦 integration-tests/gradle
✖ io.quarkus.gradle.MutableJarFormatBootsInDevModeTest.testFastJarFormatWorks
- History - More details - Source on GitHub
⚙️ Gradle Tests - JDK 17 Windows #
- Failing: integration-tests/gradle
📦 integration-tests/gradle
✖ io.quarkus.gradle.MutableJarFormatBootsInDevModeTest.testFastJarFormatWorks
- History - More details - Source on GitHub
Flaky tests - Develocity
⚙️ JVM Tests - JDK 17
📦 extensions/opentelemetry/deployment
✖ io.quarkus.opentelemetry.deployment.metrics.HttpServerMetricsTest.collectsHttpRouteFromEndAttributes
- History
-
Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds.
-org.awaitility.core.ConditionTimeoutException
-
Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds.
-org.awaitility.core.ConditionTimeoutException
-
Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds.
-org.awaitility.core.ConditionTimeoutException
There was no discussion, there was an existing way to remove classes when packaging, so I just used that. |
This PR add removing all the non-relevant dev mode classes from Prod (Dev UI Json RPC Services and HotReloadSetup etc.)
The way that you add a JSON-RPC Service in Dev UI:
You will do a similar thing in Prod mode only:
This will remove the JsonRPC service when Prod mode is build, and (due to the true) also any other classes in that package.