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

Support extension registry when adding or listing extensions in plugins #10062

Merged
merged 3 commits into from
Jul 21, 2020
Merged

Support extension registry when adding or listing extensions in plugins #10062

merged 3 commits into from
Jul 21, 2020

Conversation

gastaldi
Copy link
Contributor

@gastaldi gastaldi commented Jun 16, 2020

This PR introduces support for querying the extension registry compiled from https://github.com/quarkusio/quarkus-extension-catalog.

It will fallback to the original behavior if the registry URL is not specified.

To test:

Make sure to change the Quarkus BOM dependency in the project to a released version which is also present in the registry(eg. 1.5.1.Final)

Maven

mvn quarkus:list-extensions -DsearchPattern=kubernetes -Dregistry=https://github.com/quarkusio/quarkus-extension-catalog/releases/download/v23/registry.json

Gradle

gradle listExtensions --searchPattern=mutiny --registry=https://github.com/quarkusio/quarkus-extension-catalog/releases/download/v23/registry.json

Fixes #9593

@gastaldi gastaldi requested a review from maxandersen June 16, 2020 19:33
@boring-cyborg boring-cyborg bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/maven area/platform Issues related to definition and interaction with Quarkus Platform area/gradle Gradle area/dependencies Pull requests that update a dependency file labels Jun 16, 2020
Copy link
Contributor

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

No tests?

I know it's a still draft :-)

@maxandersen maxandersen requested a review from aloubyansky June 24, 2020 05:50
.dependabot/config.yml Outdated Show resolved Hide resolved
@maxandersen
Copy link
Member

I'm trying it out now using https://github.com/maxandersen/registrytest but I'm getting No extension found with this pattern using mvn quarkus:list-extensions -DsearchPattern=kubernetes -Dregistry=https://github.com/quarkusio/quarkus-extension-catalog/releases/download/v23/registry.json

what am I missing ?

@gastaldi
Copy link
Contributor Author

I'm trying it out now using maxandersen/registrytest but I'm getting No extension found with this pattern using mvn quarkus:list-extensions -DsearchPattern=kubernetes -Dregistry=https://github.com/quarkusio/quarkus-extension-catalog/releases/download/v23/registry.json

what am I missing ?

For some reason the platform used in your project is detected as 1.5.2.Final, that's why it doesn't show anything. I'm investigating why that is happening

@aloubyansky
Copy link
Member

I don't see anything wrong here. I understand it's a work in progress.

@maxandersen
Copy link
Member

I don't see anything wrong here. I understand it's a work in progress.

well goal was to have it in 1.6 behind a feature flag

@gastaldi gastaldi marked this pull request as ready for review June 29, 2020 18:36
@maxandersen
Copy link
Member

could we cut back on that massive registry.json ? dont think we need that big a file to test ;)

@maxandersen
Copy link
Member

maxandersen commented Jul 2, 2020

suggestions from call 2020-07-02 👍

  • 1) add "delta" info to platform and releases to capture what is different from the top level extension description
  • 2) make sure semantics of which data ends up in top level extensions and platforms are well defined (like "latest release")
  • 3) versions suggested to be "core-versions" and be a map instead of array.

"No extension found with pattern '%s' while searching in the configured extension registries (using Quarkus Core %s).\n"
+ "Falling back to existing platform extensions",
search, quarkusVersion);
platformExtensions = searchExtensions(invocation, search);
Copy link
Member

Choose a reason for hiding this comment

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

What is happening here? What is this fallback about?

Copy link
Member

Choose a reason for hiding this comment

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

Why are we having two sources of extensions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is happening here? What is this fallback about?
Why are we having two sources of extensions?

The fallback happens to make sure that the search is also performed in the platform BOM that is associated to the project. This will likely happen if the specified registry does not list the Quarkus core version used in the project

Copy link
Member

Choose a reason for hiding this comment

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

The fact that they aren't unified is strange to me. Perhaps I'm misunderstood the use-case.

@gastaldi
Copy link
Contributor Author

While trying to unify the ExtensionRegistry (like requested in #10062 (comment)), it turns out that ExtensionManager needs to be revisited to perform the installation operations (BOMs, dependencies that belong to a platform, etc), that's why the tests are failing. I'm moving this PR to Draft state until I get this sorted out

@gastaldi gastaldi marked this pull request as draft July 11, 2020 01:40
@gastaldi gastaldi marked this pull request as ready for review July 19, 2020 02:01
@gastaldi
Copy link
Contributor Author

Looks like I managed to sort out the ExtensionManager bugs and also handled the special Extension query cases

@gastaldi gastaldi marked this pull request as draft July 19, 2020 12:43
@gastaldi gastaldi marked this pull request as ready for review July 20, 2020 18:47
@gastaldi gastaldi requested a review from ia3andy July 20, 2020 18:47
@gastaldi gastaldi dismissed ia3andy’s stale review July 21, 2020 04:29

Changes addressed

final DependencyManagement managed = project.getModel().getDependencyManagement();
return managed != null ? managed.getDependencies()
: Collections.emptyList();
private List<Dependency> getManagedDependencies(MavenArtifactResolver resolver) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

FYI @ia3andy this is going to help collect platform BOMs from the parents.

Support ExtensionRegistry in AddExtensions

Support multiple registries

Introduced registry-descriptor and API

Introduced ExtensionPredicate

Add Extensions now use ExtensionRegistry.lookup

Fixes #9593
@gastaldi gastaldi merged commit 15c0e7d into quarkusio:master Jul 21, 2020
@gastaldi gastaldi deleted the registry branch July 21, 2020 23:28
@gsmet gsmet modified the milestone: 1.7.0 - master Jul 28, 2020
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 area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle area/maven area/platform Issues related to definition and interaction with Quarkus Platform release/noteworthy-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add initial implementation of list/add/remove extension utilizing registry
5 participants