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

Warn users when using older GraalVM or Mandrel versions #39866

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Apr 3, 2024

Although we try to keep backwards compatibility with older GraalVM or
Mandrel versions, these versions should not be considered as fully
supported by the users, as the compatibility might break at any
time (even at a minor level patch).

Although we try to keep backwards compatibility with older GraalVM or
Mandrel versions, these versions should not be considered as fully
supported by the users, as the compatibility might break at any
time (even at a minor level patch).
@quarkus-bot quarkus-bot bot added the area/core label Apr 3, 2024
@zakkak
Copy link
Contributor Author

zakkak commented Apr 3, 2024

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.

Good call!

Copy link
Contributor

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

Good move. I'd be even more aggressive on the minimum version, but that's probably for another day.

/**
* The minimum version of GraalVM supported by Quarkus.
* Versions prior to this are expected to cause major issues.
*/
public static final Version MINIMUM = VERSION_22_2_0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be 23.0 (JDK 17-based), by now for quarkus main. 22.2.0 is very old. Failing that, use 22.3.0 at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why though? By bumping MINIMUM we (mandrel team) will no longer be able to do tests with these versions (mostly to see the evolution through time or to see in which version a regression was introduced) even if there is no major "breakage". So far there is no extra effort required on our side to keep this as is. cc @galderz

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this being tested anywhere with Quarkus main, so unless you can show this actually works with 22.2, there is no point in keeping it.

Copy link

quarkus-bot bot commented Apr 3, 2024

Status for workflow Quarkus CI

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

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


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 integration-tests/virtual-threads/grpc-virtual-threads

io.quarkus.grpc.example.streaming.VertxVirtualThreadTest.testEmpty - History

  • INTERNAL: Half-closed without a request - io.grpc.StatusRuntimeException
io.grpc.StatusRuntimeException: INTERNAL: Half-closed without a request
	at io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:268)
	at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:249)
	at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:167)
	at io.grpc.testing.integration.TestServiceGrpc$TestServiceBlockingStub.emptyCall(TestServiceGrpc.java:211)
	at io.quarkus.grpc.example.streaming.VirtualThreadTestBase.testEmpty(VirtualThreadTestBase.java:25)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1018)

@zakkak zakkak added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Apr 3, 2024
@geoand geoand merged commit cced273 into quarkusio:main Apr 4, 2024
49 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Apr 4, 2024
@quarkus-bot quarkus-bot bot added this to the 3.10 - main milestone Apr 4, 2024
@zakkak zakkak deleted the 2024-04-03-print-warning-when-not-using-recommended-version branch April 4, 2024 10:56
zakkak added a commit to zakkak/quarkus that referenced this pull request Apr 4, 2024
@gsmet gsmet modified the milestones: 3.10 - main, 3.9.3 Apr 9, 2024
gsmet pushed a commit to gsmet/quarkus that referenced this pull request Apr 9, 2024
gsmet pushed a commit to gsmet/quarkus that referenced this pull request Apr 9, 2024
@gsmet gsmet modified the milestones: 3.9.3, 3.8.4 Apr 9, 2024
gsmet pushed a commit to gsmet/quarkus that referenced this pull request Apr 9, 2024
@gsmet gsmet removed this from the 3.8.4 milestone Apr 11, 2024
@gsmet gsmet added this to the 3.9.3 milestone Apr 11, 2024
ketola pushed a commit to ketola/quarkus that referenced this pull request May 1, 2024
@gsmet gsmet modified the milestones: 3.9.3, 3.8.5 May 15, 2024
gsmet pushed a commit to gsmet/quarkus that referenced this pull request May 15, 2024
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.

6 participants