-
Notifications
You must be signed in to change notification settings - Fork 3k
Avoids some WARNINGs from io.smallrye.common.process.Logging #48959
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
Conversation
@dmlloyd at the risk of asking a stupid question in public, I would like to know why we have Smallrye Process in Quarkus in the first place? It seems it doesn't wrap any Windows quirks JDK 17 Process API has anyway. Or does it...? If there are any benefits for more complex dev mode scenarios or something like that, why don't we use it only there? |
The problem is that it is nearly impossible to properly handle I/O and threading correctly (as evidenced by many places which were incorrect, some of which already caused hangs and other issues). The JDK API is deceptively simple, but there are many subtleties not accounted for. |
@dmlloyd Thanks. I'm glad the dependency does something more then. Do you think it would be justified to go to the Smallrye Process and refactor it so as it "doesn't do this with stdErr" or do we deal with it in Quarkus case by case? |
It is working as designed; in most cases, the subprocess will output errors on the error stream, and thus the useful information is gathered into the exception or logged if an exception is not thrown. Processes which don't do this are the exception, not the rule. |
Status for workflow
|
Status | Name | Step | Failures | Logs | Raw logs | Build scan |
---|---|---|---|---|---|---|
✖ | Native Tests - Windows support | Build |
Failures | Logs | Raw logs | 🔍 |
You can consult the Develocity build scans.
Failures
⚙️ Native Tests - Windows support #
- Failing: integration-tests/liquibase
📦 integration-tests/liquibase
✖ Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project quarkus-integration-test-liquibase: Failed to build quarkus application
Flaky tests - Develocity
⚙️ JVM Tests - JDK 17
📦 extensions/resteasy-reactive/rest-client/deployment
✖ io.quarkus.rest.client.reactive.stork.StorkResponseTimeLoadBalancerTest.shouldUseFasterService
- History
expected: "hello, Alice" but was: "hello, I'm a slow server"
-org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError:
expected: "hello, Alice"
but was: "hello, I'm a slow server"
at io.quarkus.rest.client.reactive.stork.StorkResponseTimeLoadBalancerTest.shouldUseFasterService(StorkResponseTimeLoadBalancerTest.java:58)
at java.base/java.lang.reflect.Method.invoke(Method.java:569)
at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:534)
at io.quarkus.test.QuarkusUnitTest.interceptTestMethod(QuarkusUnitTest.java:448)
This Liquidbase failure actually seems serious: Would be weird if this PR is the cause, but I'll try to reproduce it locally nonetheless.
|
And the Javadoc of |
Yes, I have an open PR that hopefully fixes this |
Got the same error here: #48598 |
#48954 will hopefully fix that |
😃 I did not realize I am talking to the very father of the initiative [1] here, DML. Running test suites on various platforms over the years, we accumulated a collection of hacks, traversing and killing all child processes first, remembering PID and calling Windows wmic and task kill in cmd rather than relying on Java API, having a tiny C executable that attaches to a detached terminal process and sends it CtrlC, to be able to gracefully stop Quarkus in some Windows environments etc., properly consuming, forwarding or getting rid off streams... I'll have a look if anything could be worth generalizing and contributing to SmallRye. [1] #48223 |
This should have been backported, marking for backport. |
yes please |
The problem is that io.smallrye.common.process treats non empty stdErr as something to print a WARNING about.
At best case it prints the stdErr twice. A worse scenario is a confusion caused for log scanning tools such as Mandrel integration testsuite that suddenly report benign things like:
i.e. it makes a WARNING out of a perfectly fine podman output.
This PR scratches the itch for some cases, but the Quarkus codebase seems riddled wit Smallrye process now, so perhaps a more general solution would be in order.