Skip to content

Conversation

holly-cummins
Copy link
Contributor

See (painful) discussion on #47892 (comment). Having invested the brainpower in figuring out what's going on, I decided I should add some comments to help future developers (most likely future me), and rename a few things to maybe make things slightly clearer.

In general, the shutdownTasks.add(doTheAction) pattern confuses me every time I hit it, because I assume doTheAction is producing something which happens at shutdown. That's sort of true, but it misses the massive side effect of doTheAction happening now. So the doTheAction method name is referring to what's happening now, not what's happening at shutdown, even though it's wrapped in shutdownTasks.add. The names and structure all just end up being a bit misleading. I don't see an alternative to the pattern, though. I guess you could pass the shutdownTasks in to the method, and then it would be self-cleaning. That's no good for the case where we do cleanup in a finally block, though.

This comment has been minimized.

@holly-cummins holly-cummins marked this pull request as draft May 16, 2025 12:35
@holly-cummins holly-cummins removed the request for review from aloubyansky May 16, 2025 12:35
@holly-cummins holly-cummins force-pushed the improve-clarity-of-profile-config-reading branch from 2072c8d to 047a8ed Compare May 16, 2025 18:18
@holly-cummins holly-cummins marked this pull request as ready for review May 16, 2025 18:18
Copy link

quarkus-bot bot commented May 17, 2025

Status for workflow Quarkus CI

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

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
Native Tests - Spring ⚠️ Check → Logs Raw logs 🚧

Flaky tests - Develocity

⚙️ 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)

@aloubyansky aloubyansky merged commit dcef943 into quarkusio:main May 19, 2025
71 of 73 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.24 - main milestone May 19, 2025
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