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

Devfile Registry Should Serve Pictures #14844

Closed
tsmaeder opened this issue Oct 10, 2019 · 10 comments
Closed

Devfile Registry Should Serve Pictures #14844

tsmaeder opened this issue Oct 10, 2019 · 10 comments
Labels
area/devfile-registry kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.

Comments

@tsmaeder
Copy link
Contributor

We currently have no place to serve the images from that represent the devfiles in the meta.yaml in the devfile registry.
Since these svg files are often developed together, we should find a way to serve those files from the devfile registry and we should be able to reference them in a non-absolute way from the meta.yaml file.
Currently, we reference the images at their github location, but abusing Github as a content delivery network seems asking for trouble.
See here for examples: https://github.com/redhat-developer/codeready-workspaces/tree/master/dependencies/che-devfile-registry/devfiles (PR's pending)

@tsmaeder tsmaeder added the kind/enhancement A feature request - must adhere to the feature request template. label Oct 10, 2019
@tsmaeder
Copy link
Contributor Author

cc @nickboldt as this is probably crw 2.0-relevant.

@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 Oct 10, 2019
@amisevsk
Copy link
Contributor

If there's any info on how Theia would handle a relative path, I can implement this no problem. My main issue is knowing what to put for the icon/picture path so that Theia will be able to process it.

@tsmaeder
Copy link
Contributor Author

@amisevsk I don't think theia handle devfile images, that would be more the dashboard, right? I think it would make sense to do the same "relative url" syntax as in html?

@tsmaeder tsmaeder added area/dashboard area/devfile-registry severity/P1 Has a major impact to usage or development of the system. and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Oct 10, 2019
@tsmaeder
Copy link
Contributor Author

P1, because crw 2.0-relevant

@nickboldt
Copy link
Contributor

nickboldt commented Oct 10, 2019

what's the current "devfile provides no image or image can't be loaded" fallback image? is it the greyed-out Che logo? Asking because we need to reskin that too and if it's no an asset in the devfile registry, it must be coming from the dashboard css somewhere... cc: @vitaliy-guliy another question about white labelling. Added to checklist in #14110 (comment)

@amisevsk
Copy link
Contributor

amisevsk commented Oct 11, 2019

I don't think theia handle devfile images, that would be more the dashboard, right? I think it would make sense to do the same "relative url" syntax as in html?

AFAICT the only place images listed in devfiles (the icons) are used is in Theia; the dashboard doesn't show images for plugins currently.

The concern with relative URLs is that I don't think the workspace is pulling from the devfile registry directly, and is instead being fed config from Che itself -- what does a relative URL refer to in this case?

Given that the devfile stored in the workspace config is the single source of truth, I don't know a good way to host images in a way that isn't very fragile.

edit: misunderstanding on my part.

@amisevsk
Copy link
Contributor

amisevsk commented Nov 4, 2019

@tsmaeder I don't think there's a good way to get the devfile registry to serve pictures currently, since every time it's deployed it can be with a different URL. A prerequisite for this would be some way to handle the relative links we'd be putting in the devfile registry (e.g. if I put ./my-icon for an icon, the dashboard tries to get the image from eclipse-che-url.com/dashboard/my-icon). I think the dashboard would need to handle it as a special case.

WDYT?

@tsmaeder
Copy link
Contributor Author

tsmaeder commented Nov 5, 2019

@amisevsk I agree that there is no short-term fix here: the underlying problem is that the plugin registry is just passively serving files which are interpreted 100% by the consumer. If we ever want to scale the registry (to thousands of plugins), we'll need to have a proper API (for example, to search).
Since we control both ends of the communication, I don't see a problem with asking consumers to interpret relative url's for the time being.

@amisevsk
Copy link
Contributor

amisevsk commented Nov 5, 2019

@tsmaeder I created issue #15091 for provisioning a registry's public endpoint as an env var to support this; if we could rely on knowing what our URL is, I could implement this without much trouble, even without a proper API (we already do it for the devfile registry, for cached project zips).

The issue with consumers interpreting relative URLs is that we would still need to pass the registry URL around -- Theia gets a devfile from Che; does it know where that devfile came from?

@sleshchenko
Copy link
Member

It seems to be fixed now https://github.com/eclipse/che-devfile-registry/tree/master/images

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devfile-registry kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

5 participants