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

Improve the add-extensions partial match approach #2737

Closed
emmanuelbernard opened this issue Jun 5, 2019 · 12 comments
Closed

Improve the add-extensions partial match approach #2737

emmanuelbernard opened this issue Jun 5, 2019 · 12 comments
Assignees
Labels
area/infra internal and infrastructure related issues kind/enhancement New feature or request triage/out-of-date This issue/PR is no longer valid or relevant

Comments

@emmanuelbernard
Copy link
Member

emmanuelbernard commented Jun 5, 2019

Today the following algorithm is used:

For each entry in -Dextensions do the following:

  1. look for extensions with exactly matching tags. e.g. a dep with tag abracadabra will match -Dextensions=abracadabra
  2. look for exensions whose name includes the query in lowercase e.g. Quarkus AbrAcadabra will match -Dextensions=brac
  3. look for exensions whose artifact id name includes the query in lowercase e.g. quarkus-abracadabra will match -Dextensions=brac
  4. if there is more than one extension matching, then none is picked and the user sees the list
  5. any full GAV is added

Right know it creates conflicts:

  • you must use the GAV to select quarkus-hibernate-orm because there is quarkus-hibernate-orm-panache
  • you must use the GAV to select arc because quarkus-hibernate-search-elasticsearch conrtains arc

I propose the following refinements:

A search command whether via start.quarkus.io or the -Dsearch would keep the previous algorithm as it maximizes the number of relevant matching entries

The -Dextensions algorithm should aim at:

  1. allowing partial typing to find a match
  2. limit to resort to GAV to disambiguate.

Here is a set of change proposal that we can partially or entirely take

  1. remove search for tags: I'd like to see when a search via tag would lead to a unique extension match
  2. make an exact artifactId match have priority and lead to a single selection even if the algorithm otherwise find several matches
    • consider foobar an exact match of quarkus-foobar
    • but if two extensions with the same artifactId and two different groupId are matching, ask the user to disambiguate

Rule 2 would offer a way out of two conflicts mentioned above:

  • someone writing arc would not have to select the GAV
  • someone writing orm would have to choose between panache and normal ORM
  • someone writing hibernate-orm would select orm and not panache
@emmanuelbernard emmanuelbernard added the kind/enhancement New feature or request label Jun 5, 2019
@emmanuelbernard
Copy link
Member Author

@gsmet Here are my rule change proposal to limit name conflicts and offer a simpler way out than having to write the GAV. I'm interesting to see what hope you can poke at it.

@gsmet
Copy link
Member

gsmet commented Jun 6, 2019

Looks good to me. Let's try to tackle that for 0.17.0.

@gsmet gsmet added this to the 0.17.0 milestone Jun 6, 2019
@FroMage
Copy link
Member

FroMage commented Jun 6, 2019

LGTM too

@emmanuelbernard
Copy link
Member Author

I'm adding a new use cases to it. Some extensions have a very very bad "SEO" because their artifactId is long and painful to write down. It would lead to other extensions or approaching being de facto preferred instead of what would be the more natural path for a Quarkus app.
To fix that, we want to explore the notion of a shortname for extensions (to be added in the JSON file).

That short name is:

  • globally unique across all extensions, or at least as unique as artifactId are,
  • ?should shortname be unique within the artifact id scope to?

Short name should not be abuse and use only when it's important. A good artifactId is preferred.
Short name match would be exposed to the user experience in two ways possible ways:

  1. it would qualify as an exact match having priority over partial matches like artifactId is (see above)
  2. it would not qualify as a exact match having priority but rather be the default proposed choice in the list of options shown
    • this means that the CLI (maven, gradle and CLI) need to be adapted to offer a prompt choice upon ambiguity and propose the short name as the default choice when applicable
    • on the website, the shortname would be exposed by it being return first in the auto-completion list upon partial match

Let's take an example.

  • io.quarkus:quarkus-smallrye-reactive-messaging-kafka
  • io.quarkus:quarkus-kafka-streams

with the shortname kafka be assigned to quarkus-smallrye-reactive-messaging-kafka

-Dextensions=kafka would offer

  • in 1. to select io.quarkus:quarkus-smallrye-reactive-messaging-kafka automatically for the user
  • in 2. to offer both but preselectionning io.quarkus:quarkus-smallrye-reactive-messaging-kafka

-Dextensions=kafka-streams would offer io.quarkus:quarkus-kafka-streams

CC @cescoffier @n1hility

@emmanuelbernard
Copy link
Member Author

On a side note, I wonder if we should consider quarkus-smallrye- as an optional prefix just like quarkus- is today. Or whether that would lead to problems.

@kenfinnigan
Copy link
Member

Could do. In theory there shouldn't be overlap, otherwise it means we're duplicating effort in different places

@emmanuelbernard
Copy link
Member Author

I not following your effort duplication argument @kenfinnigan

@kenfinnigan
Copy link
Member

If we have a quarkus-smallrye-{something} and a quarkus-{something} then it likely means {something} is implemented both in SmallRye and Quarkus, which would be duplicating effort.

@emmanuelbernard
Copy link
Member Author

Ah I get your meaning now.

@cescoffier
Copy link
Member

I'm on it, trying to see the corner cases.

@cescoffier cescoffier self-assigned this Jun 7, 2019
@gsmet gsmet modified the milestones: 0.17.0, 0.18.0 Jun 19, 2019
cescoffier added a commit to cescoffier/quarkus that referenced this issue Jun 19, 2019
…quarkusio#2737.

Impact of the new selection algorithm on the guides.

Most of it is purely cosmetic and allow using shorter names. Some tweaks are required to avoid ambiguity.

Do not use dash in bean validation.

Remove the swagger-ui short name. Swagger may mean something else.
@gsmet gsmet modified the milestones: 0.18.0, 0.17.0 Jun 19, 2019
@gsmet
Copy link
Member

gsmet commented Jun 19, 2019

I'm keeping the issue open for now as I'm not sure everything was implemented as it was supposed to be.

Feel free to close it if everything is in order.

gsmet pushed a commit that referenced this issue Jun 19, 2019
…#2737.

Impact of the new selection algorithm on the guides.

Most of it is purely cosmetic and allow using shorter names. Some tweaks are required to avoid ambiguity.

Do not use dash in bean validation.

Remove the swagger-ui short name. Swagger may mean something else.
@cescoffier
Copy link
Member

Closing as this has been fixed a long time agon

@cescoffier cescoffier added the area/infra internal and infrastructure related issues label Sep 22, 2019
@gsmet gsmet added the triage/out-of-date This issue/PR is no longer valid or relevant label Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/infra internal and infrastructure related issues kind/enhancement New feature or request triage/out-of-date This issue/PR is no longer valid or relevant
Projects
None yet
Development

No branches or pull requests

5 participants