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

Introduce registryUrl field for cheEditor/chePlugin components #13104

Closed
sleshchenko opened this issue Apr 10, 2019 · 10 comments
Closed

Introduce registryUrl field for cheEditor/chePlugin components #13104

sleshchenko opened this issue Apr 10, 2019 · 10 comments
Assignees
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. severity/P1 Has a major impact to usage or development of the system. status/in-progress This issue has been taken by an engineer and is under active development.

Comments

@sleshchenko
Copy link
Member

sleshchenko commented Apr 10, 2019

Description

This issue is created as a result of a discussion that got a place in the following issue #13062 and continued in this one.

It's not obvious that id field of chePlugin/cheEditor component may contain registry URL. And to make it clearer the format should be changed to the following

  type: chePlugin/cheEditor
  alias: editor # optional
  # the first way to specify plugin
  id: eclipse/che-theia/next # The format is {PUBLISHER}/{NAME}/{VERSION}
  registryUrl: https://che-plugin-registry.openshift.io # optional
  # the second way to specify plugin
  reference: https://raw.githubusercontent.com/eclipse/che-test/test/meta.yaml

So, user is able to specify plugin/editor with id and optional registry URL (default is default is missing) and then reference will be evaluated as {REGISTRY_URL}/plugins/{PUBLISHER}/{NAME}/{VERSION}.
Or user is able to specify reference to point to meta.yaml raw content available via HTTP and then id and registryURL must be not specified. See examples:

  • Github raw content service
  type: chePlugin/cheEditor
  reference: https://raw.githubusercontent.com/eclipse/che-plugin-registry/master/plugins/ms-vscode.node-debug/1.32.1/meta.yaml
  • Pastebin:
  type: chePlugin/cheEditor
  reference: https://pastebin.com/raw/kYprWiNB

Depends on #11818

@sleshchenko
Copy link
Member Author

@l0rd Could you please validate that it is consistent with our discussion

cc @skabashnyuk @metlos @mshaposhnik @garagatyi

@skabashnyuk skabashnyuk added kind/task Internal things, technical debt, and to-do tasks to be performed. severity/P1 Has a major impact to usage or development of the system. team/platform labels Apr 10, 2019
@l0rd
Copy link
Contributor

l0rd commented Apr 11, 2019

@sleshchenko I like that

@garagatyi
Copy link

@sleshchenko when we use plugin registry in Che we do not specify /plugins in its path since root is used by UD for getting index of all the plugins and it is API of registry too. Because of that /plugins seems like a API path that should be used by convention. If you want to change that can you elaborate how it should we used by UD when we add support of custom registries to it?

@l0rd
Copy link
Contributor

l0rd commented Apr 15, 2019

@garagatyi you mean that you would prefer

registryUrl: https://che-plugin-registry.openshift.io # optional

rather than

registryUrl: https://che-plugin-registry.openshift.io/plugins # optional

and that plugins would be implicit by convention?

@garagatyi
Copy link

No, I'm OK to change that. I just mean that we do that ATM and we should think how it will work with UD and WS master when we change that.
Now we configure registry in WS master without /plugins. UD accesses it without /plugins. Then interested sites should know about /plugins (treat it as API path). With the change should we change what WSMaster, UD does? This is what I'm interested about

@garagatyi
Copy link

In a case of pastebin or any other raw content hosting URL:

"id":"5f74f5d",
"registry":"https://pastebin.com"

if an error occurs showing user "Downloading of meta information of plugin with ID 5f74f5d failed" seems weird.
Splitting https://pastebin.com/5f74f5d to

"id":"5f74f5d",
"registry":"https://pastebin.com"

is not intuitive in my opinion.
I would rather use direct URL in reference field as @sleshchenko suggested. If you don't like reference field for id + registry case we could use reference vs id+registry fields.

@garagatyi
Copy link

we have another convention that to download meta.yaml of a plugin we need to add /meta.yaml to the end of the plugin URL constructed from registry + id + version. I think it is better to remove this convention too since it complicates things and and make different cases of meta hosting not cohesive.
For example in a case of pastebin we should not add /meta.yaml to download it, but in a case of a default or custom registry we need to.
Taking all of that into account I like

reference: https://pastebin.com/5f74f5d
reference: https://gist.githubusercontent.com/sleshchenko/e132bd9d37e8cf8f2c0a2b4929ecda3e/raw/9c28962cc3a24dcaf357b5716d98068734f51165/meta.yaml
reference: eclipse/che-theia/next

or

reference: https://pastebin.com/5f74f5d
reference: https://gist.githubusercontent.com/sleshchenko/e132bd9d37e8cf8f2c0a2b4929ecda3e/raw/9c28962cc3a24dcaf357b5716d98068734f51165/meta.yaml
id: eclipse/che-theia/next
id: eclipse/che-theia/next
registry: https://che-plugin-registry.openshift.io/plugins

@sleshchenko
Copy link
Member Author

For example in a case of pastebin we should not add /meta.yaml to download it, but in a case of a default or custom registry we need to.

Actually, in case of plugin registry use used to use it and we do not have to. The following link is valid https://che-plugin-registry.openshift.io/plugins/che-dummy-plugin/0.0.1/

I would rather use direct URL in reference field as @sleshchenko suggested. If you don't like reference field for id + registry case we could use reference vs id+registry fields.

Both options of reference or reference vs id+registry fields look good to me. Let me list here examples for format with three fields reference, id, registryUrl and user is able to specify reference or id + registryUrl

  reference: https://pastebin.com/raw/kYprWiNB
  id: eclipse/che-theia:next # required. The format is {PUBLISHER}/{NAME}:{VERSION}
  registryUrl: https://che-plugin-registry.openshift.io/ # optional
  # reference should not be specified and it will be evaluated as `{registryUrl}/{publisher}/{name}/{version}`

@garagatyi Please correct me if I'm wrong here

@l0rd Can you share your opinion about it?

@l0rd
Copy link
Contributor

l0rd commented Apr 16, 2019

@sleshchenko I am ok with your proposal of using reference or id + registryUrl (optional)

@sleshchenko
Copy link
Member Author

The issue description is changed according to the latest agreement about reference or id + registryUrl (optional) fields

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. severity/P1 Has a major impact to usage or development of the system. status/in-progress This issue has been taken by an engineer and is under active development.
Projects
None yet
Development

No branches or pull requests

5 participants