-
Notifications
You must be signed in to change notification settings - Fork 85
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
Add support for configuring devfile registry #49
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems OK for me.
I'll rebase the work I started on issue redhat-developer/rh-che#1476 on master when this PR is merged.
That's related to endgame issue eclipse-che/che#13557 |
Successfully tested it on Openshift V4. LGTM. |
@@ -71,7 +73,7 @@ func GetCustomConfigMapData() (cheEnv map[string]string) { | |||
"CHE_INFRA_KUBERNETES_SERVICE__ACCOUNT__NAME": "che-workspace", | |||
"CHE_WORKSPACE_AUTO_START": "true", | |||
"CHE_INFRA_KUBERNETES_WORKSPACE__UNRECOVERABLE__EVENTS": "FailedMount,FailedScheduling,MountVolume.SetUp failed,Failed to pull image", | |||
"CHE_LIMITS_WORKSPACE_IDLE_TIMEOUT": "-1", | |||
"CHE_LIMITS_WORKSPACE_IDLE_TIMEOUT": "-1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weird indent here but not a showstopper
@@ -28,6 +28,7 @@ const ( | |||
DefaultIngressClass = "nginx" | |||
DefaultPluginRegistryUrl = "https://che-plugin-registry.openshift.io" | |||
DefaultUpstreamPluginRegistryUrl = "https://che-plugin-registry.openshift.io/v3" | |||
DefaultDevfileRegistryUrl = "https://che-devfile-registry.openshift.io" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will these eventually change to be part of the Che deployment, once the registries are local to the deployment rather than shared instances on openshift.io?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will change that in a next PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with one minor formatting question and one followup work question
This is to support che-incubator/chectl#114 (implemented in che-incubator/chectl#189).