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

easy add/modify registry list #18775

Open
maxandersen opened this issue Jul 16, 2021 · 11 comments
Open

easy add/modify registry list #18775

maxandersen opened this issue Jul 16, 2021 · 11 comments
Assignees
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins kind/enhancement New feature or request

Comments

@maxandersen
Copy link
Member

Description

for now users will need to know where to put quarkus config to setup registries. we should make this dead easy.

Here are examples with suggestions for syntax to explain the various cases:

given a empty config:

quarkus registry add registry.acme.org

now all operations (like quarkus create) should use registry.acme.org for resolving platforms/extensions and if cannot be found it should error and not try fallback to registry.quarkus.io as user explicilty asked to only use registry.acme.org.

doing this next then:

quarkus registry add registry.quarkus.io

then when using quarkus create or other operations it will take registry.acme.org into concernfirst and then secondary regstry.quarkus.io

quarkus registry clear
quarkus registry add registry.quarkus.io registry.acme.org

will clear the list and now add registry.quarkus.io in front making it the dominant default.

this implies that if user only adds registry.acme.org it will not be able to see community provided extensions UNLESS reigstry.acme.org has a copy of such in its metadata.

Implementation ideas

No response

@maxandersen maxandersen added the kind/enhancement New feature or request label Jul 16, 2021
@maxandersen maxandersen added area/cli Related to quarkus cli (not maven/gradle/etc.) area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins labels Jul 16, 2021
@maxandersen
Copy link
Member Author

@ebullient does this help about the notion of registry and sequence?

@aloubyansky can you confirm the semantics i described above matches your expections/what we have implemented at the moment?

@aloubyansky
Copy link
Member

aloubyansky commented Jul 16, 2021

Yes, I thought we agreed on failing instead of silently falling back previously but also printing a message about how to install a platform using -P.

I'd be careful with quarkus registry clear.

@maxandersen
Copy link
Member Author

Yes. The commands should help where it can with -P info.

Clear is my suggestion so we have minimal set of commands to allow easily add to config but also able to reset to change order easily.

Btw. We might also want to have a --global / --local or some mechanism to say where the registry should be created/updated.

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 17, 2021

/cc @ebullient, @gsmet

@ebullient
Copy link
Member

Ok. Intent is clear, but rammifications aren't.
Each registry appears in the config as a block of attributes:

registries:
- registry.acme.org:
    custom:
      prop: "value"
    maven:
      repository:
        url: "https://repo.acme.org/maven"

etc. etc.

If I have customized any of these, I don't want any command that just changes the search/priority order of registries to wipe out those customizations.

I've started #18769 to experiment with making registries: a map, such that we can use the insertion order in the absence of anything else, but could introduce another simple list of ordered ids for prededence.

e.g. registries becomes the catalog of registry configurations (so we don't have to copy/duplicate, and can actually have a durable/predictable merge), and order determination becomes registry.precedence or something.

  • If you have nothing, we use registry.quarkus.io
  • We use registry.acme.org If you specify the following:
    registries:
      registry.acme.org: 
    
  • We use registry.quarkus.io If you specify the following:
    registries:
      registry.acme.org:
        disabled: true
    
  • Given the following, we would use registry.acme.org then registry.quarkus.io:
    registry.precedence:
    - registry.acme.org
    - registry.quarkus.io
    registries:
      registry.quarkus.io: 
      registry.acme.org: 
    

@maxandersen
Copy link
Member Author

why would we need a registry.precedence ? isn't LinkedHashmap used to back these?

why not just reorder the list/map ? or are you just suggesting registry.precedence as optional alternative ?
a caveat with that is that then you need to decide what you do when registry.precedence and registries does not match.

maybe a list is better:

registries:
  - id: registry.acme.org
  - id: registry.quarkus.io

@aloubyansky
Copy link
Member

A list is better, imo. Which is why I would keep the current impl for now.

@maxandersen
Copy link
Member Author

Ah yes. Current impl is a list already. +1 for that being the most meaningful.

@ebullient
Copy link
Member

ebullient commented Jul 19, 2021

Regardless, the Map is backed by a LinkedHashMap (as you mentioned, Max), so in the simplest cases, you wouldn't need an additional field, you would use map insertion order (which is what I showed above). The question is what happens if you want to change the search order without losing config.

What I'm confused about is how durable we think this registry configuration is (because some of the attributes I can see being things you don't want to mess around with). The existing mixed-use list (both the order AND the config) doesn't work if you want a command like clear to be used to remove all registries as in your initial example, Max, because that would also drop all of the associated config (maven repo urls, enabled flags, allowed package filters, ... )

One way to interpret add/remove is based on adding/removing configured registries:

  • registry add should ... add a registry configuration (minimally that's just the registry id):
    • --global would add it to the global config (default)
    • --local would add it to the project config (which would allow a project to override settings from global config, e.g. the update policy)
  • registry remove should .. remove the configuration of the registry entirely (?)
    • --global would remove the registry from the global config (if present, default)
    • --local would... remove a local override (if present)

or should we use define/undefine for that?

Or is none of this necessary because the extra registry config that we're carting around is what is unnecessary.. :-P

@aloubyansky
Copy link
Member

Up until this issue, we haven't really considered actually merging multiple configs. There is no notion of a config hierarchy currently. The first config file found is what we use, atm. Not saying it must stay this way but that's how it is today, just in case.

The original idea behind the config was it was supposed to be super light and simple, i.e. a list of a few URLs/IDs. For a typical user we don't even expect the need for this config. So I am not sure we should be targeting more complex scenarios at this point.

Structure wise, what we currently have is:

registries:
  - registry.acme.com
  - registry.other.org:
      enabled: false

If eventually we want to be able to extract the registry config out of the list (to prevent completely loosing the custom registry configs executing registry clear) we could introduce another attribute, e.g.

registries:
  - registry.acme.com
  - registry.other.org
registry-definitions:
  registry.other.org:
    custom:
      prop: "value"

registry-definitions would not be affected by registry clear. I personally also don't see problem with allowing custom config in the list as well, e.g.

registries:
  - registry.acme.com
  - registry.other.org
      enabled: false
registry-definitions:
  registry.other.org:
    custom:
      prop: "value"

Given that the current config model is compatible with the above, I don't think we should be rushing in with this change.

For now registry add/remove/enable/disable could still be supported. With add and remove simply adding IDs to the list. I am not sure we should add clear now though.

@ebullient
Copy link
Member

Partially delivered by #19619

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins kind/enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

4 participants