Skip to content

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented May 15, 2025

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/google-cloud-functions labels May 15, 2025
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Thanks, I added a few comments.

Comment on lines 51 to 52
<maven.compiler.source>17</maven.compiler.source>
<maven.compiler.target>17</maven.compiler.target>
Copy link
Member

Choose a reason for hiding this comment

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

Could you please drop this commit from there? It has nothing to do with the issue at hand and I'm not sure it's worth taking any risk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no risk at all. They are fully redundant, because they are defined once again three lines below:

        <maven.compiler.source>17</maven.compiler.source>
        <maven.compiler.target>17</maven.compiler.target>
        <maven.compiler.release>17</maven.compiler.release>
        <maven.compiler.source>${maven.compiler.release}</maven.compiler.source>
        <maven.compiler.target>${maven.compiler.release}</maven.compiler.target>

The change is in a separate commit. I can submit it through a separate PR, if you like.

pom.xml Outdated

<!-- Make sure to check compatibility between these 2 gRPC components before upgrade -->
<grpc.version>1.69.1</grpc.version> <!-- when updating, verify if com.google.auth should not be updated too -->
<grpc.version>1.69.1</grpc.version> <!-- when updating, verify if com.google.auth and perfmark.version should not be updated too -->
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, could you add a link to https://central.sonatype.com/ on the artifact where we can check the versions? Sometimes it's quite hard to figure out when you have to navigate through a couple of parent POMs.

@ppalaga
Copy link
Contributor Author

ppalaga commented May 15, 2025

2d6ff1b:

I thought you rely on dependabot for the upgrades. If you'd like to stick to the version used by grpc instead, then you may want to use the maven plugin I wrote for this. We use it in QCXF and Camel Quarkus. You can annotate a property as follows (if the version is vebatim in a dependency)

<perfmark.version>0.26.0</perfmark.version><!-- @sync io.grpc:grpc-core:${grpc.version} dep:io.perfmark:perfmark-api -->

Then run mvn org.l2x6.cq:cq-maven-plugin:4.17.2:sync-versions -N and perfmark.version gets upgraded to 0.27.0. We call the mojo on GH actions and fail the build it it causes changes in the source tree.

Copy link

quarkus-bot bot commented May 15, 2025

Status for workflow Quarkus CI

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

✅ 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 Windows

📦 extensions/micrometer-opentelemetry/deployment

io.quarkus.micrometer.opentelemetry.deployment.compatibility.MicrometerTimedInterceptorTest.testTimeMethod_AsyncFailed - History

  • Stream has no elements - java.lang.IllegalArgumentException
java.lang.IllegalArgumentException: Stream has no elements
	at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lambda$lastReading$2(MetricDataFilter.java:213)
	at java.base/java.util.Optional.orElseThrow(Optional.java:403)
	at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lastReading(MetricDataFilter.java:213)
	at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lastReadingDataPoint(MetricDataFilter.java:231)
	at io.quarkus.micrometer.opentelemetry.deployment.compatibility.MicrometerTimedInterceptorTest.testTimeMethod_AsyncFailed(MicrometerTimedInterceptorTest.java:150)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:521)

⚙️ JVM Integration Tests - JDK 17

📦 integration-tests/opentelemetry

io.quarkus.it.opentelemetry.LoggingResourceTest.testException - History

  • Condition with Lambda expression in io.quarkus.it.opentelemetry.LoggingResourceTest was not fulfilled within 2 minutes. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.LoggingResourceTest was not fulfilled within 2 minutes.
	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:1160)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1129)
	at io.quarkus.it.opentelemetry.LoggingResourceTest.testException(LoggingResourceTest.java:113)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

⚙️ JVM Integration Tests - JDK 21

📦 integration-tests/opentelemetry

io.quarkus.it.opentelemetry.LoggingResourceTest.testException - History

  • Condition with Lambda expression in io.quarkus.it.opentelemetry.LoggingResourceTest was not fulfilled within 2 minutes. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.LoggingResourceTest was not fulfilled within 2 minutes.
	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:1160)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1129)
	at io.quarkus.it.opentelemetry.LoggingResourceTest.testException(LoggingResourceTest.java:113)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

📦 integration-tests/opentelemetry-grpc-only

io.quarkus.it.opentelemetry.grpc.HelloGrpcClientTest.testHello - History

  • java.lang.RuntimeException: Failed to start quarkus - java.lang.RuntimeException
java.lang.RuntimeException: java.lang.RuntimeException: Failed to start quarkus
	at io.quarkus.test.junit.QuarkusTestExtension.throwBootFailureException(QuarkusTestExtension.java:667)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestClassConstructor(QuarkusTestExtension.java:762)
	at java.base/java.util.Optional.orElseGet(Optional.java:364)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: java.lang.RuntimeException: Failed to start quarkus
	at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)

@ppalaga
Copy link
Contributor Author

ppalaga commented May 16, 2025

@gsmet is this good to merge now? All those failed tests are flaky, aren't they?

@ppalaga
Copy link
Contributor Author

ppalaga commented May 19, 2025

Is this good to merge, @gsmet?

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Hey. I was on PTO :).

Your plugin looks interesting. What I'm not sure of when we align this sort of things is which component will drive the version used as you might have incompatibilities at some point.

I suppose we could try and see how it goes.

@gsmet gsmet merged commit bcd0cb5 into quarkusio:main May 20, 2025
57 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.24 - main milestone May 20, 2025
@ppalaga
Copy link
Contributor Author

ppalaga commented May 20, 2025

Your plugin looks interesting. What I'm not sure of when we align this sort of things is which component will drive the version used as you might have incompatibilities at some point.

Yeah, indeed, the plugin really helps only in simple cases where one needs to stick to a single specific version brought by some dependency. When a transitive comes via several paths, dependabot tends to be a better solution.

@gsmet gsmet modified the milestones: 3.24 - main, 3.23.0 May 20, 2025
@ppalaga
Copy link
Contributor Author

ppalaga commented Jun 3, 2025

@gsmet could you please explain why this is won't get backported? It would be great to have it in the next Quarkus 3.20 version to avoid some manual quirks in Camel Quarkus.

1 similar comment
@ppalaga
Copy link
Contributor Author

ppalaga commented Jun 10, 2025

@gsmet could you please explain why this is won't get backported? It would be great to have it in the next Quarkus 3.20 version to avoid some manual quirks in Camel Quarkus.

@jmartisk jmartisk modified the milestones: 3.23.0, 3.20.2 Jun 25, 2025
<protobuf-java.version>${protoc.version}</protobuf-java.version>
<protobuf-kotlin.version>4.29.3</protobuf-kotlin.version>
<proto-google-common-protos.version>2.56.0</proto-google-common-protos.version>
<perfmark.version>0.27.0</perfmark.version><!-- dependency of io.grpc:grpc-core not managed in io.grpc:grpc-bom -->
Copy link
Member

Choose a reason for hiding this comment

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

@ppalaga, to me it seems that perfmark should be managed in io.grpc:grpc-bom. Can you please create io.grpc issue/PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rsvoboda sounds legit, but the grpc project has apparently chosen to manage only their own artifacts in their BOM: https://repo1.maven.org/maven2/io/grpc/grpc-bom/1.73.0/grpc-bom-1.73.0.pom So I doubt there is any chance that they would be ready to accept such as change.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, thanks for the info

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Manage io.perfmark:perfmark-api to keep Quarkus, Camel Quarkus, Quarkus CXF and Quarkus Google Cloud Services in sync
4 participants