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

[#1414] Upgrade Quarkus to 2.6.1.Final and fix metadata of extension #1415

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gsmet
Copy link
Contributor

@gsmet gsmet commented Jan 1, 2022

Fixes #1414

Note: there's a good chance CI will fail as the license headers need an update but I'll let you do that yourself as the license update also removed a new line on my box and I'm not sure it's expected.

Let me know when I can rebase this one.

return null;
}
return new CapabilityBuildItem(CAPABILITY);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's worth keeping that for new versions given Quarkus 2 is there for quite a while now and there's very little chance people would use new versions of the extension with it.

@@ -78,6 +78,25 @@
</resource>
</resources>
<plugins>
<plugin>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-bootstrap-maven-plugin</artifactId>
Copy link
Contributor Author

@gsmet gsmet Jan 1, 2022

Choose a reason for hiding this comment

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

@aloubyansky can you check I did things properly? I checked the metadata and they look fine now.

Metadata are:

---
name: "Blaze-Persistence"
metadata:
  short-name: "BP"
  keywords:
  - "jpa"
  - "hibernate"
  - "blaze-persistence"
  - "entity-view"
  - "dto"
  - "mapper"
  - "sql"
  guide: "https://quarkus.io/guides/blaze-persistence"
  categories:
  - "data"
  status: "stable"
  built-with-quarkus-core: "2.6.1.Final"
  capabilities:
    provides:
    - "com.blazebit.persistence.integration.quarkus"
  extension-dependencies:
  - "io.quarkus:quarkus-core"
  - "io.quarkus:quarkus-hibernate-orm"
  - "io.quarkus:quarkus-agroal"
  - "io.quarkus:quarkus-arc"
  - "io.quarkus:quarkus-datasource"
  - "io.quarkus:quarkus-narayana-jta"
  - "io.quarkus:quarkus-mutiny"
  - "io.quarkus:quarkus-smallrye-context-propagation"
  - "io.quarkus:quarkus-caffeine"
artifact: "com.blazebit:blaze-persistence-integration-quarkus::jar:1.6.5-SNAPSHOT"
description: "Advanced SQL support for JPA and Entity-Views as efficient DTOs"

@aloubyansky
Copy link

Looks good @gsmet, thanks!

@famod
Copy link

famod commented Jan 3, 2022

Coming here from quarkiverse/quarkus-logging-manager#125

First load of Dev UI breaks with blaze 1.6.4 on Quarkus 2.6.1. Without that lazy optimization from quarkusio/quarkus@315f6b2 (which I first thought was the cause) the error pops up eagerly during boot of dev mode (which is even more unpleasant).

@famod
Copy link

famod commented Jan 3, 2022

...aaand it breaks on Quarkus 2.5.4 as well (eagerly this time because that lazy tweak was added only to 2.6.0.CR1+).

@aloubyansky
Copy link

@famod i think @gsmet's PR may fix it.

@famod
Copy link

famod commented Jan 3, 2022

@aloubyansky yeah, thats what I reckoned.

It feels there is a test coverage gap here. I mean quarkus integration wise 1.6.4 is kinda doomed, 1.6.5 will probably fix it.
But tbh, I would expect a red light to go on before a release of an extension is published? Is this gap a general problem or just specific to blaze?

No offense really, it's just that 1.6.4 took a while and so it's a bit sad to see it turn out like this.

@aloubyansky
Copy link

Extensions can include dev mode tests in their test suite (and the the platform one), I suppose that would catch this kind of issue.

@gsmet
Copy link
Contributor Author

gsmet commented Jan 3, 2022

TBH, my main interrogation is why it didn't fail earlier.

@aloubyansky
Copy link

Earlier - meaning 1.6.3?

@famod
Copy link

famod commented Jan 3, 2022

1.6.3 didn't have this: #1376 (but not sure if related)

@gsmet
Copy link
Contributor Author

gsmet commented Jan 3, 2022

Outch, yes, so that might be it because it removes the metadata generation. My guess is that the quarkus-extension.properties file should also be removed from the repo? WDYT @aloubyansky ?

@aloubyansky
Copy link

Yes, if the plugin is present anyway, it's not worth it having the descriptor as a static resource.

@gsmet
Copy link
Contributor Author

gsmet commented Jan 3, 2022

Added a commit to remove it.

@famod
Copy link

famod commented Jan 3, 2022

Turns out there is at least one QuarkusDevModeTest already...but it seems all tests are running with Quarkus 1.11.0.Final!

@gsmet
Copy link
Contributor Author

gsmet commented Jan 3, 2022

I also updated the Quarkus version in this PR.

@famod
Copy link

famod commented Jan 3, 2022

Right, I missed that one.
FTR: So with 2.5.4.Final that one test fails, but due to the lazy tweak in 2.6, 2.6.1.Final doesn't fail (because the test is apparently not calling dev ui).

@gsmet
Copy link
Contributor Author

gsmet commented Jan 4, 2022

Error:    bad class file: /home/runner/.m2/repository/io/quarkus/arc/arc/2.6.1.Final/arc-2.6.1.Final.jar(io/quarkus/arc/DefaultBean.class)
Error:      class file has wrong version 55.0, should be 52.0

So now we have the issue that Quarkus 2 is JDK 11 only.

@beikov beikov added this to the 1.6.5 milestone Jan 4, 2022
@beikov beikov added component: quarkus kind: bug worth: high Implementing this has a high worth labels Jan 4, 2022
@beikov beikov unassigned gsmet Jan 4, 2022
@beikov beikov removed kind: bug component: quarkus worth: high Implementing this has a high worth labels Jan 4, 2022
@beikov beikov removed this from the 1.6.5 milestone Jan 4, 2022
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.

Upgrade Blaze Persistence to Quarkus 2.6.1.Final and generate proper metadata
4 participants