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

Propagate quarkusRegistryClient property #249

Merged
merged 3 commits into from
Nov 23, 2023

Conversation

jsmrcka
Copy link
Contributor

@jsmrcka jsmrcka commented Nov 22, 2023

The property tells Quarkus Maven Plugin whether it should use the online registry to resolve extension catalog. Default is true for Quarkus. Thanks to the propagation, it can now be overriden when running tests by using -DquarkusRegistryClient=false.

Fixes
#115.

Additional fixes:

  • Decrease combinations size in native GH test
    • To avoid OutOfMemoryError for increased number of extensions (174 at the
      moment).
  • Exclude camel-quarkus-mybatis and logging-json
    • camel-quarkus-mybatis needs a datasource.
    • There is a name collision between io.quarkus:quarkus-logging-json and io.quarkiverse.loggingjson:quarkus-logging-json.

The property tells Quarkus Maven Plugin whether it should use the online
registry to resolve extension catalog. Default is `true` for Quarkus.
Thanks to the propagation, it can now be overriden when running tests by
using `-DquarkusRegistryClient=false`.

Fixes
quarkus-qe#115.
@jsmrcka jsmrcka requested a review from jedla97 November 22, 2023 11:25
@jsmrcka jsmrcka self-assigned this Nov 22, 2023
Copy link
Member

@jedla97 jedla97 left a comment

Choose a reason for hiding this comment

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

Thanks for resolving it! LGTM!

@jedla97
Copy link
Member

jedla97 commented Nov 22, 2023

Not look on all tests but some of them failing because of logging-json which have two version

- io.quarkus:quarkus-logging-json
- io.quarkiverse.loggingjson:quarkus-logging-json

To avoid OutOfMemoryError for increased number of extensions (174 at the
moment).
@rsvoboda
Copy link
Member

@jsmrcka How is this gonna work when new branch is introduced?

When we for example introduce 3.7 branch, will it fetch list of extensions for 3.7 Quarkus stream and main will be using Quarkus latest?

@jsmrcka
Copy link
Contributor Author

jsmrcka commented Nov 22, 2023

@jsmrcka How is this gonna work when new branch is introduced?

When we for example introduce 3.7 branch, will it fetch list of extensions for 3.7 Quarkus stream and main will be using Quarkus latest?

@rsvoboda Extensions are being listed using this POM: https://github.com/quarkus-qe/quarkus-extensions-combinations/blob/main/src/main/resources/list-extensions.pom.xml, which imports a BOM based on root POM of the TS (or on mvn arguments, if provided).
Different branches usually have their own Quarkus version, so for each one, the list of extensions should correspond to the Quarkus stream.

This PR does not change that.

@rsvoboda
Copy link
Member

rsvoboda commented Nov 22, 2023

So the command is operating on top of generated list-extensions.pom.xml, so the right stream gets picked from the bom defined there.

I still see some challenges:
Upstream:

Downstream:

  • planning to use dedicated registry or using -DquarkusRegistryClient=false ?

@jsmrcka
Copy link
Contributor Author

jsmrcka commented Nov 22, 2023

Downstream:

  • planning to use dedicated registry or using -DquarkusRegistryClient=false ?

Yes, the -DquarkusRegistryClient=false. See jenkins-jobs/-/merge_requests/747.

Copy link
Member

@rsvoboda rsvoboda left a comment

Choose a reason for hiding this comment

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

CI is green now

@rsvoboda rsvoboda merged commit a4f8967 into quarkus-qe:main Nov 23, 2023
4 checks passed
@rsvoboda
Copy link
Member

@jsmrcka jsmrcka deleted the quarkus-registry-client branch November 23, 2023 09:47
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.

3 participants