Skip to content
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

Fix Quarkus platform BOM version info collection for analytics #45487

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

aloubyansky
Copy link
Member

@aloubyansky aloubyansky commented Jan 9, 2025

Pixes platform BOM info collection processing project's dependency version constraints

@aloubyansky aloubyansky requested a review from brunobat January 9, 2025 20:20
@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform labels Jan 9, 2025
@aloubyansky aloubyansky requested a review from rsvoboda January 9, 2025 20:20

This comment has been minimized.

Copy link
Member

@rsvoboda rsvoboda left a comment

Choose a reason for hiding this comment

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

LGTM and thanks for the fix @aloubyansky.

@brunobat about CUSTOM value of the version - will the version reports need to be adjusted to accommodate this addition or it will be transparent to them?

@aloubyansky
Copy link
Member Author

I didn't start a separate discussion about this. CUSTOM is a suggestion. FYI @maxandersen

Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

I think CUSTOM is fine and will show up in the versions on future reports.
This change must not be back-ported because it will pollute previous data in the system, @gsmet, @aloubyansky

@gsmet
Copy link
Member

gsmet commented Jan 10, 2025

This change must not be back-ported because it will pollute previous data in the system

I'm not sure I understand? For me, it's just make the reporting a bit more accurate but shouldn't pollute anything?

Could you clarify?

@aloubyansky
Copy link
Member Author

Yeah, I am not sure about the risk of backporting. But if you don't like the CUSTOM change, backporting the main fix would make sense. I could create a dedicated PR for 3.15 in that case.

@brunobat
Copy link
Contributor

brunobat commented Jan 10, 2025

Because data for existing back-ported versions will have a mix of N/A and CUSTOM that will clash with the previous logic of only having N/A, especially if we look to major.minor versions.
If we do it from now on, we will know that those versions data is always consistent.
We can backport, but we will need to filter by full version in the case of 3.15.x if we require consistent data...
... Or maybe we don't need to be that strict and just need understanding what's going on.

@brunobat
Copy link
Contributor

Yeah, I am not sure about the risk of backporting. But if you don't like the CUSTOM change, backporting the main fix would make sense. I could create a dedicated PR for 3.15 in that case.

I like the CUSTOM, and it can be on a different PR.

@aloubyansky
Copy link
Member Author

Just to clarify, @brunobat you would like me to remove the CUSTOM change from this one and add it in a separate PR?

@brunobat
Copy link
Contributor

Yes, that way we can back-port the remaining changes without any concerns.

@aloubyansky aloubyansky force-pushed the analytics-quarkus-version branch from 8c986a6 to 3e7ebdf Compare January 10, 2025 11:19
@aloubyansky
Copy link
Member Author

The CUSTOM change is now in #45501

Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

Excellent! Thanks @aloubyansky

Copy link

quarkus-bot bot commented Jan 10, 2025

Status for workflow Quarkus CI

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

✅ 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

📦 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:256)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)

📦 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)

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/opentelemetry-quickstart

io.quarkus.it.opentelemetry.OpenTelemetryDisabledTest.buildTimeDisabled - History

  • Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryDisabledTest was not fulfilled within 200 milliseconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryDisabledTest was not fulfilled within 200 milliseconds.
	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 io.quarkus.it.opentelemetry.OpenTelemetryDisabledTest.buildTimeDisabled(OpenTelemetryDisabledTest.java:29)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

@aloubyansky aloubyansky merged commit d379a5e into quarkusio:main Jan 10, 2025
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Jan 10, 2025
@gsmet gsmet modified the milestones: 3.18 - main, 3.17.7 Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform triage/backport-3.15 triage/flaky-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants