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

Migrate to Quarkus 3 and Java 17 #23

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

beatngu13
Copy link

This PR migrates the Quarkus Petclinic to Quarkus 3 and Java 17.

I did the Quarkus 3 upgrade with the quarkus-maven-plugin. Afterwards, I was facing the following build error:

Caused by: io.quarkus.builder.BuildException: 
Build failure: Build failed due to errors
	[error]: Build step io.quarkus.rest.client.reactive.deployment.RestClientReactiveProcessor#addRestClientBeans threw an exception: java.lang.RuntimeException: The class jakarta.ws.rs.core.Application is not inside the Jandex index
	at org.jboss.resteasy.reactive.common.processor.JandexUtil.isImplementorOf(JandexUtil.java:397)
	at io.quarkus.rest.client.reactive.deployment.RestClientReactiveProcessor.addRestClientBeans(RestClientReactiveProcessor.java:526)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:909)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:282)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
	at java.base/java.lang.Thread.run(Thread.java:833)
	at org.jboss.threads.JBossThread.run(JBossThread.java:501)

Which is why I added a quarkus.index-dependency entry for jakarta.ws.rs:jakarta.ws.rs-api.

For the Java 17 upgrade, I only modified the pom.xml so far. There are still these Java 11 references:

  1. https://github.com/redhat-developer-demos/quarkus-petclinic/blob/master/devfile.yaml#L17
  2. https://github.com/redhat-developer-demos/quarkus-petclinic/blob/master/src/main/docker/Dockerfile.jvm#L26
  3. https://github.com/redhat-developer-demos/quarkus-petclinic/blob/master/src/main/docker/Dockerfile.fast-jar#L26

Unfortunately, I'm not quite sure which (Red Hat) images / packages to use for Java 17. Therefore, I marked this PR as a draft.

Comment on lines 24 to 25
quarkus.index-dependency.jakarta-ws-rs-api.group-id=jakarta.ws.rs
quarkus.index-dependency.jakarta-ws-rs-api.artifact-id=jakarta.ws.rs-api
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really understand why this change. Can you elaborate?

Copy link
Author

@beatngu13 beatngu13 Jul 4, 2023

Choose a reason for hiding this comment

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

Me neither 🤷‍♂️ When I did the upgrade:

$ ./mvnw io.quarkus.platform:quarkus-maven-plugin:3.1.3.Final:update -N

The build (./mvnw verify) was failing afterwards with the error mentioned in the PR description:

Caused by: io.quarkus.builder.BuildException: 
Build failure: Build failed due to errors
	[error]: Build step io.quarkus.rest.client.reactive.deployment.RestClientReactiveProcessor#addRestClientBeans threw an exception: java.lang.RuntimeException: The class jakarta.ws.rs.core.Application is not inside the Jandex index
	at org.jboss.resteasy.reactive.common.processor.JandexUtil.isImplementorOf(JandexUtil.java:397)
	at io.quarkus.rest.client.reactive.deployment.RestClientReactiveProcessor.addRestClientBeans(RestClientReactiveProcessor.java:526)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:909)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:282)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
	at java.base/java.lang.Thread.run(Thread.java:833)
	at org.jboss.threads.JBossThread.run(JBossThread.java:501)

I'm not familiar with Jandex, so I tried following this guide from the docs. To my understanding, the jandex-maven-plugin doesn't help in this case (tried it anyway, indeed didn't help). But adding this quarkus.index-dependency entry fixed the error.

Do you have an explanation for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if you remove the properties?

Copy link
Author

Choose a reason for hiding this comment

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

As I said, the aforementioned error is thrown.

To illustrate this, I have created a reproducer in a new branch quarkus3-java17-no-props based on this PR. First, I let GitHub Actions build all branches:

Then, I remove the two properties:

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will try and have a look tomorrow

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is actually a bug in Quarkus itself for which I have opened a fix.

Hopefully the fix will land in Quarkus 3.2.2 at which time we can move this PR forward

geoand added a commit to geoand/quarkus that referenced this pull request Jul 6, 2023
This would happen when @Blocking was used on a class
that extended Jakarta REST's Application class
and the Reactive REST Client extension was present

The issue was originally reported at:
redhat-developer-demos/quarkus-petclinic#23
gsmet pushed a commit to gsmet/quarkus that referenced this pull request Jul 18, 2023
This would happen when @Blocking was used on a class
that extended Jakarta REST's Application class
and the Reactive REST Client extension was present

The issue was originally reported at:
redhat-developer-demos/quarkus-petclinic#23

(cherry picked from commit 0cfba8c)
@beatngu13
Copy link
Author

@geoand updated Quarkus 3.1.3 to 3.2.2, which includes your fix (landed in 3.2.1). Build works fine now. 👍

IMO what's left for this PR to be merge-ready:

For the Java 17 upgrade, I only modified the pom.xml so far. There are still these Java 11 references:

  1. https://github.com/redhat-developer-demos/quarkus-petclinic/blob/master/devfile.yaml#L17
  2. https://github.com/redhat-developer-demos/quarkus-petclinic/blob/master/src/main/docker/Dockerfile.jvm#L26
  3. https://github.com/redhat-developer-demos/quarkus-petclinic/blob/master/src/main/docker/Dockerfile.fast-jar#L26

Unfortunately, I'm not quite sure which (Red Hat) images / packages to use for Java 17.

@geoand
Copy link
Collaborator

geoand commented Jul 31, 2023

Nice!

As for the Java 17 image, you can use java-17-openjdk-headless

@beatngu13 beatngu13 marked this pull request as ready for review July 31, 2023 09:00
@beatngu13
Copy link
Author

Hi @geoand,

Is there anything I can do, so we can move forward with this PR?

@geoand
Copy link
Collaborator

geoand commented Mar 2, 2024

Sorry, this feel off my radar completely!

Mind updating to 3.8.1 now that we have released it?

Copy link
Collaborator

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks!

@beatngu13
Copy link
Author

Looks like the native image build is failing, will look into that.

The DDL of `vet_specialties` is:

```sql
create table vet_specialties (
    specialty_id bigint not null,
    vet_id bigint not null,
    primary key (specialty_id, vet_id)
)
```

The `import.sql` was creating tuples of `(vet_id, specialty_id)`, causing the following exception on startup:

```text
org.postgresql.util.PSQLException: ERROR: insert or update on table "vet_specialties" violates foreign key constraint "fk35uiboyrpfn1bndrr5jorcj0m"
  Detail: Key (specialty_id)=(1004) is not present in table "specialties".
```
@beatngu13
Copy link
Author

The error message:

 Caused by: com.oracle.graal.pointsto.constraints.UnsupportedFeatureException: Discovered unresolved method during parsing: org.quarkus.samples.petclinic.owner.Pet.getVisits(). This error is reported at image build time because class org.quarkus.samples.petclinic.owner.Pet_ValueResolver is registered for linking at image build time by command line and command line.

This similar to quarkusio/quarkus#6396, but that has been resolved in Quarkus 1.2.0 … will further investigate.

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

Successfully merging this pull request may close these issues.

2 participants