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

Update apicurio-common-rest-client-vertx version #26322

Merged
merged 1 commit into from
Jun 29, 2022

Conversation

edeandrea
Copy link
Contributor

@edeandrea edeandrea commented Jun 23, 2022

Update apicurio-common-rest-client-vertx version to the most recent version. The current version (0.1.9.Final) does not work with some versions of apicurio. 0.1.11.Final does, and is being used already. See https://github.com/quarkusio/quarkus-super-heroes/blob/280ba7d522d8f3a30a809cd92c8c89987276d8f9/rest-fights/pom.xml#L104-L122

Specifically see #25378 (comment)

Closes #25378

Update `apicurio-common-rest-client-vertx` version to the most recent version. The current version (`0.1.9.Final`) does not work with some versions of apicurio. `0.1.11.Final` does, and is being used already. See https://github.com/quarkusio/quarkus-super-heroes/blob/280ba7d522d8f3a30a809cd92c8c89987276d8f9/rest-fights/pom.xml#L104-L122

Closes quarkusio#25378
@gsmet
Copy link
Member

gsmet commented Jun 27, 2022

@ozangunalp could you have a look at this small one? I'd like to backport it in time for 2.10.1.Final. Thanks!

@ozangunalp
Copy link
Contributor

ozangunalp commented Jun 28, 2022

@edeandrea The change itself LGTM.

Overall this is also related to #25814 pointed out by @pjgg.

Between apicurio-registry 2.1.5 and 2.2.4 there has been a breaking change that makes Quarkus integration (via rest-client-vertx) not backwards compatible. (I remember that there was a discussion somewhere but can't find it now)

IIRC the update of apicurio-common-rest-client-vertx to 0.1.9.Final was to have a compatible version with apicurio-registry 2.2.x.

So now it would seem there has been another breaking change?

I'll run some checks using the existing IT to confirm different incompatibilities.
But I doubt that there is an easy solution. I fear that we need to accept the(se) breaking change(s) and document it somewhere.

@edeandrea
Copy link
Contributor Author

@ozangunalp I can dig up the conversation I had with @carlesarnal if you'd like to review it. It was offline on an internal chat.

Maybe updating to 0.1.9 is good enough? Maybe that's the incompatibility you're talking about.

Do you know when the update to 0.1.9 happened? Was it after Quarkus 2.7.x?

@carlesarnal
Copy link
Contributor

Hi there, I'll add some comments just for clarification.

@edeandrea The change itself LGTM.

Overall this is also related to #25814 pointed out by @pjgg.

Between apicurio-registry 2.1.5 and 2.2.4 there has been a breaking change that makes Quarkus integration (via rest-client-vertx) not backwards compatible. (I remember that there was a discussion somewhere but can't find it now)

What happened in this version was that a new request is being made in the client libraries so, if you run an older version of the server than the one used in the client libraries, it will fail.

IIRC the update of apicurio-common-rest-client-vertx to 0.1.9.Final was to have a compatible version with apicurio-registry 2.2.x.

So now it would seem there has been another breaking change?

No breaking changes here. A bug was introduced in 0.1.9.Final causing the integration to fail and it's fixed in 0.1.11.Final. In any case, the second version is the one being used in Apicurio Registry 2.2.4.Final, just as the comment in the pom says, those versions must be aligned (although in most cases it should be ok to use a different one, in this case, it should be updated because of the bug).

I'll run some checks using the existing IT to confirm different incompatibilities. But I doubt that there is an easy solution. I fear that we need to accept the(se) breaking change(s) and document it somewhere.

I hope this clarifies things a bit, and, if you have any questions or concerns, just ping me.

Thanks!

@ozangunalp
Copy link
Contributor

Ok thanks for the clarification @carlesarnal !
Let's pinpoint a working version of apicurio-common-rest-client-vertx for apicurio-registry version 2.1.5 (2.1.x maybe ?) and document it as @pjgg suggested.

From the bom history I can tell 0.1.5 was working with 2.1.5.

@gsmet
Copy link
Member

gsmet commented Jun 29, 2022

@ozangunalp why are we targeting 2.1.5 for main? Shouldn't we target the latest version?

@ozangunalp
Copy link
Contributor

@gsmet we are not targeting 2.1.5 for the main, we are already depending on 2.2.4.

In #25814 @pjgg was suggesting that we document this as a breaking change. Because using a recent version of Quarkus with apicurio registry version <= 2.1.5 you need to force a compatible version of apicurio-common-rest-client-vertx.

@edeandrea
Copy link
Contributor Author

@ozangunalp when you say "pinpoint" are you saying it's only a documentation thing to say after apicurio 2.1.5.Final there is a breaking change and that you need to force a version of apicurio-common-rest-client-vertx? Or are you saying that Quarkus should be based on 2.1.5.Final, using apicurio-common-rest-client-vertx version 0.1.5?

@ozangunalp
Copy link
Contributor

It's a documentation thing to say if someone would like to use Quarkus with apicurio-registry version <= 2.1.5 they'll have to force versions of apicurio-rest-client.

@edeandrea
Copy link
Contributor Author

Got it. Thanks for clarifying.

@edeandrea
Copy link
Contributor Author

Would you like me to document it as part of this PR?

@ozangunalp
Copy link
Contributor

@edeandrea actually no, before the explanation from @carlesarnal I thought that there was more to it. But I'll update the doc in a separate PR and close #25814. Thanks!

@gsmet gsmet merged commit 4fcbf03 into quarkusio:main Jun 29, 2022
@quarkus-bot quarkus-bot bot added this to the 2.11 - main milestone Jun 29, 2022
@edeandrea edeandrea deleted the patch-2 branch July 1, 2022 18:41
@gsmet gsmet modified the milestones: 2.11 - main, 2.10.2.Final Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file kind/bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't hook app into Red Hat managed apicurio service
4 participants