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

Allow resolver API to resolve other files than [.]devfile.yaml #19540

Closed
Tracked by #18668
benoitf opened this issue Apr 8, 2021 · 14 comments
Closed
Tracked by #18668

Allow resolver API to resolve other files than [.]devfile.yaml #19540

benoitf opened this issue Apr 8, 2021 · 14 comments
Assignees
Labels
area/che-server kind/task Internal things, technical debt, and to-do tasks to be performed. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes
Milestone

Comments

@benoitf
Copy link
Contributor

benoitf commented Apr 8, 2021

Is your task related to a problem? Please describe.

Today, dashboard is using the resolver REST API to get devfile.yaml. It supports github, gitlab, bitbucket, etc and avoids to clone repository to get files in a pretty fast way.
But we do need to parse extra files like .che/che-editor.yaml , .che/che-theia-plugins.yaml or .vscode/extensions.json

Describe the solution you'd like

Today dashboard is calling resolver API with url parameter of the 'devfile location'
https://github.com/eclipse/che-dashboard/blob/d611b71241fbba4f625cf33abd1ba42002ff889c/src/store/FactoryResolver.ts#L74

https://github.com/eclipse/che-workspace-client/blob/9a0680107d86524c31aa63548a05713c4f47a574/src/rest/resources.ts#L194

https://github.com/eclipse/che/blob/889187e4e1218ee769e0327e8fb27d1aa45109be/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java#L80

and then it gets data.devfile with the content of the devfile

So here, we could add extra argument like the other files that we may want to extract with relative path from the git repository of the devfile

In addition of url, provide extraFiles, files being an optional array of string (or any other format/parameter name)

and then we can have content of files in returned data
like data.files.<name-of-file>.content providing the content of the file and data.files..status saying if the file was there or not (or having content being undefined)

Describe alternatives you've considered

Feel free to update resolver method, as long as we can provide extra files to get and get the content (if file exist) in the result, it's 👍

Additional context

Part of DevWorkspace

@benoitf benoitf added kind/task Internal things, technical debt, and to-do tasks to be performed. area/che-server labels Apr 8, 2021
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Apr 8, 2021
@benoitf benoitf added the new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes label Apr 8, 2021
@l0rd
Copy link
Contributor

l0rd commented Apr 8, 2021

Added as a subtask of #18668

@rhopp rhopp removed the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Apr 9, 2021
@skabashnyuk skabashnyuk added this to the 7.31 milestone May 5, 2021
@mshaposhnik
Copy link
Contributor

After internal team discussion, we agreed that custom file resolver has nothing common to factory, so it's more logical to have it in separated service, as example:

GET .../api/git/resolver?repository=https://gitlab.com/myproject/myrepo&file=/subfolder/plugins.yaml

@benoitf
Copy link
Contributor Author

benoitf commented May 7, 2021

hello @mshaposhnik will that be efficient ?
It means that we'll need at least two REST API calls and on the server it will compute more a less two times the same objects to grab the content ?

What about these two usecases:

  1. if I resolve a factory with https://github.com/eclipse/che/tree/7.30.x, does it mean we call resolver API with api/git/resolver?repository=https://github.com/eclipse/che/tree/7.30.x ? (then repository parameter looks odd)

  2. if I have a factory URL like https://github.com/eclipse/che/blob/master/devfile.yaml, what should I give later to resolver service ?
    api/git/resolver?repository=https://github.com/eclipse/che/blob/master/devfile.yaml ?

if so then I don't see why it's not part of the existing resolver service because having to pass a link to a devfile.yaml to grab other resources is counter-intuitive to me

(Having like query parameters for file like that won't reach URL limit ? )

@mshaposhnik
Copy link
Contributor

My thought based on fact that factory resolver service returns FactoryDto object, which may (or may not) contain the Devfile inside. If we want additionally return content of the random extra file, how (and why?) it should be integrated into factory object?
So yes, idea is having separate calls to resolve factory with devfile by calling /factory/resolver and then fetch any additional files by calling /git/resolver or whatever. But maybe my vision of initial idea is broken :)

@benoitf
Copy link
Contributor Author

benoitf commented May 7, 2021

if the problem is on the returned object then maybe yes a new method can handle that

if instead of calling /factory/resolver we call /devfile/resolver (and it will return content of all requested files + create devfile.yaml content if no devfile and/or update devfile content with project url if not defined) then yes it might be easier to not touch the existing FactoryDto method

@skabashnyuk
Copy link
Contributor

@benoitf I would like to give some context that may clarify @mshaposhnik 's proposal. We want to build a service that can return any file from git/scm repository. It is unlinked from the che factory. In case if we think that performance is critical here we always can go with JSON RPC and Websocket.

@benoitf
Copy link
Contributor Author

benoitf commented May 7, 2021

@skabashnyuk yes but for usecase2 I don't see how it will work easily

The current need is for che acceptance of a devfile so it's where the initial work is expected.

If you want to provide a separate API as well, that's nice but exposing a separate API does not mean that /devfile/resolver could not delegate to another service on server side as well

@skabashnyuk
Copy link
Contributor

We want to deprecate /devfile/resolver in favor of a new service. The reason for that is that FactoryDTO is quite limited to get unstructured content. For example #19563

@benoitf
Copy link
Contributor Author

benoitf commented May 7, 2021

who will handle #19371 ? (add repository projects entry in the devfile) if not the devfile resolver then ?

@skabashnyuk
Copy link
Contributor

who will handle #19371 ? (add repository projects entry in the devfile) if not the devfile resolver then ?

Good question. I don't know.

@l0rd
Copy link
Contributor

l0rd commented May 7, 2021

Good question. I don't know.

@skabashnyuk you discussed that with @sleshchenko 1 month ago. As I remember you agreed to do that at the che devworkspace client level but you should know that better than I do.

@skabashnyuk
Copy link
Contributor

@skabashnyuk you discussed that with @sleshchenko 1 month ago. As I remember you agreed to do that at the che devworkspace client level but you should know that better than I do.

Could be. I do not clearly get the whole picture. I am more or less confident that Resolver API should not do that and should return the content of the files in original form.

@l0rd
Copy link
Contributor

l0rd commented May 11, 2021

We had a call this morning and considered that milestone 2 due date is the 19th of June and that retrieving the editor and theia plugins from the git repository is a fundamental feature of that milestone we should:

  • temporarily use current factory API to get those new files
  • refactor the che-server resolver API and analyse performances as part of devfile v2 milestone 3

So next step is for @mshaposhnik to propose a change of the current API that could be consumed by the dashboard to retrive these new files. The proposal should be discussed next monday during the DevWorkspace cabal call.

@mshaposhnik
Copy link
Contributor

So the following scheme is proposed and not meet any objections:

  • Project information as a separate field;
  • Separate file resolver service, with ready-to-use links to it included in factory resolve response.

Screenshot_1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/che-server kind/task Internal things, technical debt, and to-do tasks to be performed. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes
Projects
None yet
Development

No branches or pull requests

6 participants