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

Naming convention #55

Closed
l0rd opened this issue Nov 5, 2018 · 47 comments
Closed

Naming convention #55

l0rd opened this issue Nov 5, 2018 · 47 comments
Labels
enhancement New feature or request

Comments

@l0rd
Copy link
Contributor

l0rd commented Nov 5, 2018

There is no proper naming convention for Che plugins yet. This is issue is to propose one. The goal is to make something simple but still powerful, more similar to the VS Code extensions naming convention than Java packages.

  • A plugin should be identified by 3 fields:
    • publisher-id: usually matches the github organisation or user
    • plugin-id: usually matches the github repository
    • version: in the form MAJOR.MINOR.PATCH and using the Semantic Versioning
  • publisher-id and plugin-id should be lowercase and less than 64 chars (each). Only letters, digits and hyphen are allowed (LDH rule).
  • A plugin should be go in the folder: plugins/publisher_id/plugin_id/version/
@l0rd l0rd added the enhancement New feature or request label Nov 5, 2018
@tsmaeder
Copy link
Contributor

tsmaeder commented Nov 5, 2018

  1. How does a person or organization get a publisher-id?
  2. Can you please give a concrete example of publisher-id?

@benoitf
Copy link
Contributor

benoitf commented Nov 5, 2018

We discussed as well to remove some stuff from yaml file (id, version) as it's duplicating the directory layout, should it be part of this enhancement or another one ?

@benoitf
Copy link
Contributor

benoitf commented Nov 5, 2018

@tsmaeder

  1. as it's based on folders, if folder exists in https://github.com/eclipse/che-plugin-registry/tree/master/plugins then it is taken
  2. example of publisher id : redhat

@tsmaeder
Copy link
Contributor

tsmaeder commented Nov 5, 2018

1. as it's based on folders, if folder exists in https://github.com/eclipse/che-plugin-registry/tree/master/plugins then it is `taken`

So a committer decides whether to give a publisher-id to a certain when he accepts a PR with some folder name (like 'redhat')? What about squatters? How do we ensure that only redhatters can make 'redhat' plugins?

@benoitf
Copy link
Contributor

benoitf commented Nov 7, 2018

I would say it's up to reviewers. Also in meta.yaml you're referencing a repository. So probably if you try to propose redhat/cool-plugin but it ends to a repository http://github.com/thecoolguy/ then you can probably think that it's not part of a Red Hat project.

I would say that this kind of features (Creating an account, publishing on its own namespace is for marketplace, here registry is just exposing the directory layout)

@l0rd
Copy link
Contributor Author

l0rd commented Nov 7, 2018

@tsmaeder this should be a discussion about a naming convention, not about how to fight squatters. I mean if the name of a plugin is com.redhat.developers.che.helloworld or redhat-developers/che-helloworld we should still fight squatters.

@garagatyi
Copy link

garagatyi commented Mar 20, 2019

After discussing with @l0rd and @ibuziuk we decided to move files in Che plugin registry according to the suggested FS structure. We also want to add model versioning to the structure, so it would be possible to change layout of meta files model in registry just by changing model version part in the path to the meta.yaml.
For example:
Now:
plugin_ID/version/
After this issue:
/v1/plugins/publisher_id/plugin_id/version/
/v2/plugins/publisher_id/plugin_id/version/
/v3/plugins/publisher_id/plugin_id/version/
This would require checking that known components that uses plugin registry work well with the change.

Since previous Che deployments use plugin registry deployed at Openshift.io we can also add plugins folder without version prefix with current meta.yaml files and keep it for several versions until we are OK with incompatible change.
@l0rd @ibuziuk @davidfestal @amisevsk WDYT?

@garagatyi
Copy link

BTW if user created workspace with registry that uses current notation and then we update registry on Openshift.io to a new scheme user's workspaces would not start. Am I missing something?

@garagatyi
Copy link

@l0rd who should be the publisher when Red Hat employee created Che plugin entry with VS Code extension authored by Microsoft on the VS Code marketplace

@benoitf
Copy link
Contributor

benoitf commented Mar 20, 2019

I don't know if it's related to the structure but most of VS Code extensions published on the plug-in registry have the theia-endpoint:next as endpoint so they basically work only with che-theia:next

Is there a way to ensure that when we run che-theia:latest we don't see :next plugins ? (like a requirement)

or we need to think how to provide the theia endpoint without adding it in the image.

@l0rd
Copy link
Contributor Author

l0rd commented Mar 20, 2019

@l0rd who should be the publisher when Red Hat employee created Che plugin entry with VS Code extension authored by Microsoft on the VS Code marketplace

@garagatyi I would say that for a VS Code extension publisher and name in the che plugin registry should match the ones of the marketplace. Even if an employee from another company has added it to the registry

@benoitf I think the proper way is to provide the theia endpoint without adding it in the image.

@l0rd
Copy link
Contributor Author

l0rd commented Mar 20, 2019

Otherwise @garagatyi I am ok with the description of the implementation

@amisevsk
Copy link
Contributor

Me too, makes sense to me.

@garagatyi
Copy link

@benoitf we could use proper versioning to declare this dependency on the Theia version. Though we don't have it in place ATM

@benoitf
Copy link
Contributor

benoitf commented Mar 21, 2019

@garagatyi I kind of like the idea to move out the endpoint runtime from the docker image (so the image contains only the plug-in + the runtime of this plug-in but is not tied to the endpoint (so no need to inherit from custom images))

@garagatyi
Copy link

@benoitf do you mean that image should include proper version of Node.JS or you are thinking about injecting it with the plugin runner?

@benoitf
Copy link
Contributor

benoitf commented Mar 21, 2019

@garagatyi injecting it (I experimented it using nexe or pkg to make a single executable). I had to recompile nodejs for alpine to avoid the requirement on libstdc++ vercel/pkg#555). But basically we could have one binary for alpine and one binary for linux/amd64 (I don't have checked how to run it but it could be through a PV, copy or any other great idea :)

@garagatyi
Copy link

We need a plan how to migrate existing workspaces from old notation to a new one. Just versioning won't help us since users will stuck with old non-evolving plugins and changing registry prefix to v2 will break such workspaces.
We could leave existing plugins on their places and add plugins with the new notation. This would avoid breaking old workspaces after registry update. Then we need some way to deprecate these plugins and make users migrate to new ones.
We can handle that with the help of user dashboard:

  • add deprecated field to meta of a plugin
  • add migrateTo field to meta of a plugin
  • when user opens UD, UD checks plugins for deprecation and if fields are defined show warning sign to user and button to migrate plugins. It will replace old ID, version with value from migrateTo field.
    I'll also try to come up with other solutions, but feel free to comment if you have an idea on how to deal with the situation or what solution you think is not good and why.

@garagatyi
Copy link

@davidfestal @ibuziuk @amisevsk It is topic we discussed today on the standup call. Feel free to provide your ideas

@garagatyi
Copy link

@l0rd you didn't mention what should be the separator between publisher_id and plugin_id when we use it in plugins list in workspace config. I assume that version is separated by colon :.
Did you mean attributes look like:

{
  "attributes": {
      "editor": "redhat.theia-ide:2.3.4",
      "plugin": "microsoft.typescript-ls:1.0.11"
  }
}

or maybe

{
  "attributes": {
      "editor": "redhat/theia-ide:2.3.4",
      "plugin": "microsoft/typescript-ls:1.0.11"
  }
}

or maybe we should change model and put publisher id, plugin id and version in different fields, e.g.

{
  "attributes": {
  },
  "editor": {
      "publisher": "redhat",
      "id": "theia-ide",
      "version": "2.3.4"
  },
  "plugins": [ 
      {
          "publisher": "microsoft",
          "id": "typescript-ls",
          "version": "1.0.11"
      },
      {
          "publisher": "redhat",
          "id": "openshift-connector",
          "version": "4.0.8"
      }
  ]
}

@l0rd
Copy link
Contributor Author

l0rd commented Mar 22, 2019

@garagatyi your second proposal makes more sense to me. What do you think?

@garagatyi
Copy link

@l0rd yes, second option is more aligned with idea of publisher == github organization in the issue description.

@sleshchenko
Copy link
Member

To make user able to copy/paste id from the registry index and use it for his Devfile. Also, a combination of publisher and name is not identical, and publisher/name/version looks like a thing that identifies the plugin.
@garagatyi Does it make sense?

@garagatyi
Copy link

@sleshchenko yes, thanks for the information.
Interesting part here is that if we include version in a plugin ID and then update plugin registry on OSIO, UD in current Che installations would add :version to it.
In WS config it would look like:

  • ws-skeleton/che-service-plugin/0.0.1:0.0.1 for id with slash before version and new plugin
  • che-service-plugin/0.0.1:0.0.1 for id with slash before version and old plugin
  • ws-skeleton/che-service-plugin:0.0.1:0.0.1 for id with colon before version and new plugin
  • che-service-plugin:0.0.1:0.0.1 for id with colon before version and old plugin
    Which makes our changes backward incompatible and we wanted to avoid this

@garagatyi
Copy link

@ibuziuk @l0rd should we ensure that deploying new version of the registry on OSIO won't break Che
installations of previous version?

@l0rd
Copy link
Contributor Author

l0rd commented Apr 17, 2019

@sleshchenko I prefer the slash / rather than the semicolon : because then id will match the path of the meta.yaml URL. And a complete URL (gist or pastebin) could be composed with registryUrl + id. That was the rationale behind that. But now we have the reference as well so if we need to specify a meta.yaml URL a user should use that instead. So I am ok with publisher/name:version as well.

Something that we should keep in mind is that we need to implement eclipse-che/che/issues/12937 hence I would expect that 99% of the time users won't specify the version of the plugin in their devfile (they would just want latest).

@garagatyi No we do not want to break previous Che versions, that's why we should add the plugins with the new naming to the v2 folder.

@garagatyi
Copy link

Just discussed with @sleshchenko plugins upgrade policies taking into account new notation both with colon eclipse/che-theia:^0.1.0, eclipse/che-theia:~0.1.0 and with slash eclipse/che-theia/^0.1.0, eclipse/che-theia/~0.1.0 tilde and caret look a bit weird. Maybe colon is not that weird. But usage of a separate field would look better:

tool:
  id: eclipse/che-theia
  registryUrl: https://redhotchili.peppers/che-registry/
  version: ~0.1.0

@garagatyi
Copy link

But again, as Mario said id in this case is not really a unique identifier, but rather name of the plugin

@l0rd
Copy link
Contributor Author

l0rd commented Apr 18, 2019

@garagatyi @sleshchenko I know you guys like to find new challenges but we are not going to support those ~ and ^ scenarios in a devfile. Please have a look at About versions of Che 7 plugins and editors che-dev thread were we discuss about that.

Nice registryUrl name by the way 🎸

@garagatyi
Copy link

@l0rd right, sorry, I forgot about that agreement.

@garagatyi
Copy link

I'm going to use slash as version separator in new naming convention as Mario like it more and Sergii and I hesitating to choose

garagatyi pushed a commit to eclipse-che/che-plugin-broker that referenced this issue Apr 19, 2019
Details at eclipse-che/che-plugin-registry#55
Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com>
garagatyi pushed a commit to eclipse-che/che-plugin-broker that referenced this issue Apr 19, 2019
Details at eclipse-che/che-plugin-registry#55
Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com>
garagatyi pushed a commit to eclipse-che/che-plugin-broker that referenced this issue Apr 24, 2019
Make broker download metas using new naming convention
Details at eclipse-che/che-plugin-registry#55
Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com>
garagatyi pushed a commit that referenced this issue Apr 24, 2019
Copy plugins into 'v2/publisher' folders.
Rename field name to displayName.
Rename field id to name.
Remove /meta.yaml from plugin links in index.
Add deprecate section to old plugins.
Rework scripts.
Add migrate section to registry index.
Add old notation plugins to v2 not to break exisiting workspaces.
Adapt Readme.md.
Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com>
ibuziuk added a commit to ibuziuk/che-plugin-registry that referenced this issue Apr 25, 2019
…aming convention

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
ibuziuk added a commit that referenced this issue Apr 25, 2019
…tion changes

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
ibuziuk added a commit that referenced this issue Apr 25, 2019
…ntion

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
ibuziuk added a commit that referenced this issue Apr 25, 2019
…tion changes

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
ibuziuk added a commit that referenced this issue Apr 25, 2019
…s icon to the README.md

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
ibuziuk added a commit that referenced this issue Apr 25, 2019
…s icon to the README.md

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
ibuziuk added a commit to openshiftio/saas-openshiftio that referenced this issue Apr 25, 2019
Includes changes required for naming convention and vs code marketplace rate limit error:
- eclipse-che/che-plugin-registry#55
- eclipse-che/che#12840
ibuziuk added a commit to openshiftio/saas-openshiftio that referenced this issue Apr 25, 2019
Includes changes required for naming convention and vs code marketplace rate limit error:
- eclipse-che/che-plugin-registry#55
- eclipse-che/che#12840
@ibuziuk
Copy link
Member

ibuziuk commented Apr 30, 2019

@garagatyi am I correct that once eclipse-che/che#13204 is merged we can close the issue?

@garagatyi
Copy link

@ibuziuk there are 2 other PRs to adapt chectl and rh-che to the naming convention changes che-incubator/chectl#89, redhat-developer/rh-che#1390
We could track them as separate issues, but they are related to this issue, actually.

@ibuziuk
Copy link
Member

ibuziuk commented May 2, 2019

@ibuziuk ibuziuk closed this as completed May 17, 2019
monaka added a commit that referenced this issue Mar 20, 2020
Create update-to-the-upstream.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants