Deprecate workspaces with Devfile stored as workspace config #13588

mkuznyetsov · 2019-06-20T10:10:51Z

Signed-off-by: Mykhailo Kuznietsov mkuznets@redhat.com

What does this PR do?

Deprecate things related to storing workspace converted from devfile as a workspace config:

deprecated service method in DevfileService;
reworked factory-from-direct-url flow to generate factory with devfile;
FactoryDto now has devfile field, and workspace field became non-mandatory;
GitHub factories with local referenced components will be automatically resolve it to the referenceContent field of respective components (same as before);
small fixes to dashboard to support factory with devfile startup flow;

What issues does this PR fix or reference?

#13423

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

...space/src/main/java/org/eclipse/che/api/workspace/server/model/impl/devfile/DevfileImpl.java

...ace/src/main/java/org/eclipse/che/api/workspace/server/model/impl/devfile/ComponentImpl.java

…onfig Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

Ohrimenko1988 · 2019-06-21T08:09:54Z

ci-test

metlos

LGTM with 1 minor nitpick

...core-api-model/src/main/java/org/eclipse/che/api/core/model/workspace/devfile/Component.java

metlos · 2019-06-21T09:34:58Z

dashboard/src/app/factories/load-factory/load-factory.controller.ts

+      // set factory attribute:
+      let attrs = {factoryId: this.factory.id};
+
+      let creationPromise = this.cheAPI.getWorkspace().createWorkspaceFromDevfile(null, devfile, attrs);


add the the todo here as well just to be sure?

sleshchenko · 2019-06-21T08:08:40Z

...elenium-core/src/main/java/org/eclipse/che/selenium/core/factory/TestFactoryInitializer.java

+    public FactoryDto withDevfile(DevfileDto devfileDto) {
+      return factoryDto.withDevfile(devfileDto);
+    }
+
    @Override
    public WorkspaceConfigDto getWorkspace() {


Please add @nullable and describe when it may be such. The same for workspace config field

core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/factory/Factory.java

...elenium-core/src/main/java/org/eclipse/che/selenium/core/factory/TestFactoryInitializer.java

sleshchenko · 2019-06-21T11:04:07Z

As far as I know, factory used to generate a name if workspace with such name already exist, I tried to test your PR and saw this error

Please double check if it is not a regression. I mean against accepting a factory with workspace config but against the deprecated way of accepting factory with devfile via DevfileService.

sleshchenko · 2019-06-21T11:11:45Z

Have you seen that the following message is shown on a first stage of accepting a factory from Devfile?

I think it's not expected behavior, please consider fixing it or create a separate issue.

sleshchenko · 2019-06-21T11:13:57Z

One more issue I faced during manual testing: my repository had an incorrect devfile (apiVersion was 0.0.1 instead of 1.0.0) and I saw an exception from Che Server only on a phase of workspace starting, the factory was successfully accepted and in the result I had a created incorrect workspace that I was not able to start.

Please check factory accepting when Devfile is invalid, devfile should be validated on a phase of a factory accepting but not after workspace creating.
P.S. You can use the following Devfile for testing https://raw.githubusercontent.com/sleshchenko/spring-petclinic/master/devfile.yaml

che-bot · 2019-06-21T11:42:22Z

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:13588
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

mkuznyetsov · 2019-07-01T13:13:56Z

now behaviour of accepting direct URL factories will now go as follows:

if repository contains Devfile -> create workspace from this Devfile
else, if it contains factory.json -> create factory with this factory.json
else, factory will be created with base generated devfile, instead of workspace-config based workspace

also @l0rd and others, there would be slight regression, if I would attempt to use workspace from devfiles by default, for factories without devfile.yaml - there wouldn't be support for "keepDir" param for specifying subdirectory of a repository of a cloned project (devfile projects don't support "keepDir", while

Since it's a feature only used by Che 6 factory, I'd suggest fixing "keepDir" in a separate issue (which would require changes to model, schemas, etc. and probably for Theia as well, which is something that @sunix should know about )

…ctory.json Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

skabashnyuk · 2019-07-01T14:05:02Z

@mkuznyetsov yes. please create issues for keepDir support

Ohrimenko1988 · 2019-07-02T09:11:40Z

ci-test

che-bot · 2019-07-02T09:32:19Z

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:13588
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

…l or factory.json Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

mkuznyetsov · 2019-07-02T13:26:36Z

ci-test

che-bot · 2019-07-02T13:55:24Z

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:13588
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

…ile.yaml or factory.json Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

mkuznyetsov · 2019-07-02T14:35:48Z

ci-test

che-bot · 2019-07-02T17:42:00Z

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:13588
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

mkuznyetsov · 2019-07-03T07:48:27Z

ci-test

che-bot · 2019-07-03T10:59:43Z

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:13588
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

SkorikSergey · 2019-07-03T12:20:59Z

Selenium tests execution on Eclipse Che Multiuser on OCP (https://ci.codenvycorp.com/job/che-pullrequests-test-ocp/1888//Selenium_20tests_20report/) doesn't show any regression against this Pull Request.

mkuznyetsov added 2 commits June 20, 2019 11:19

Deprecate workspaces with Devfile stored as workspace config

e8c5c8d

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

fixup! Deprecate workspaces with Devfile stored as workspace config

cb6419b

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

skabashnyuk reviewed Jun 20, 2019

View reviewed changes

...space/src/main/java/org/eclipse/che/api/workspace/server/model/impl/devfile/DevfileImpl.java Outdated Show resolved Hide resolved

...ace/src/main/java/org/eclipse/che/api/workspace/server/model/impl/devfile/ComponentImpl.java Outdated Show resolved Hide resolved

fixup! fixup! Deprecate workspaces with Devfile stored as workspace c…

a70dd99

…onfig Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

mkuznyetsov marked this pull request as ready for review June 21, 2019 08:05

mkuznyetsov requested review from amisevsk, ashumilova, benoitf, dmytro-ndp, metlos, mshaposhnik, musienko-maxim, Ohrimenko1988 and sleshchenko as code owners June 21, 2019 08:05

metlos approved these changes Jun 21, 2019

View reviewed changes

sleshchenko reviewed Jun 21, 2019

View reviewed changes

core/che-core-api-model/src/main/java/org/eclipse/che/api/core/model/factory/Factory.java Show resolved Hide resolved

sleshchenko reviewed Jun 21, 2019

View reviewed changes

...elenium-core/src/main/java/org/eclipse/che/selenium/core/factory/TestFactoryInitializer.java Show resolved Hide resolved

sleshchenko mentioned this pull request Jun 21, 2019

Chectl uses deprecated method for creating workspaces from devfile che-incubator/chectl#185

Closed

mshaposhnik approved these changes Jun 21, 2019

View reviewed changes

sleshchenko mentioned this pull request Jun 24, 2019

Chectl uses deprecated method for creating workspaces from devfile #13623

Closed

benoitf approved these changes Jun 26, 2019

View reviewed changes

Merge branch 'master' of github.com:eclipse/che into che-13423

4fca0f0

che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Jun 27, 2019

mkuznyetsov added 2 commits June 27, 2019 16:45

fixup

c884713

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

Merge branch 'master' of github.com:eclipse/che into che-13423

be74539

Generate devfile for factories from GitHub without devfile.yaml or fa…

d695f21

…ctory.json Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

skabashnyuk approved these changes Jul 1, 2019

View reviewed changes

fixup! Generate devfile for factories from GitHub without devfile.yam…

102cfeb

…l or factory.json Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

fixup! fixup! Generate devfile for factories from GitHub without devf…

97d01fd

…ile.yaml or factory.json Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

Merge branch 'master' of github.com:eclipse/che into che-13423

ff19659

mkuznyetsov merged commit 8d3cc80 into master Jul 3, 2019

mkuznyetsov deleted the che-13423 branch July 26, 2019 13:31

skabashnyuk pushed a commit that referenced this pull request Jan 3, 2020

Deprecate workspaces with Devfile stored as workspace config (#13588)

bfb8a4c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate workspaces with Devfile stored as workspace config #13588

Deprecate workspaces with Devfile stored as workspace config #13588

mkuznyetsov commented Jun 20, 2019

Ohrimenko1988 commented Jun 21, 2019

metlos left a comment

metlos Jun 21, 2019

sleshchenko Jun 21, 2019

sleshchenko commented Jun 21, 2019

sleshchenko commented Jun 21, 2019

sleshchenko commented Jun 21, 2019 •

edited

Loading

che-bot commented Jun 21, 2019

mkuznyetsov commented Jul 1, 2019

skabashnyuk commented Jul 1, 2019

Ohrimenko1988 commented Jul 2, 2019

che-bot commented Jul 2, 2019

mkuznyetsov commented Jul 2, 2019

che-bot commented Jul 2, 2019

mkuznyetsov commented Jul 2, 2019

che-bot commented Jul 2, 2019

mkuznyetsov commented Jul 3, 2019

che-bot commented Jul 3, 2019

SkorikSergey commented Jul 3, 2019

Deprecate workspaces with Devfile stored as workspace config #13588

Deprecate workspaces with Devfile stored as workspace config #13588

Conversation

mkuznyetsov commented Jun 20, 2019

What does this PR do?

What issues does this PR fix or reference?

Ohrimenko1988 commented Jun 21, 2019

metlos left a comment

Choose a reason for hiding this comment

metlos Jun 21, 2019

Choose a reason for hiding this comment

sleshchenko Jun 21, 2019

Choose a reason for hiding this comment

sleshchenko commented Jun 21, 2019

sleshchenko commented Jun 21, 2019

sleshchenko commented Jun 21, 2019 • edited Loading

che-bot commented Jun 21, 2019

mkuznyetsov commented Jul 1, 2019

skabashnyuk commented Jul 1, 2019

Ohrimenko1988 commented Jul 2, 2019

che-bot commented Jul 2, 2019

mkuznyetsov commented Jul 2, 2019

che-bot commented Jul 2, 2019

mkuznyetsov commented Jul 2, 2019

che-bot commented Jul 2, 2019

mkuznyetsov commented Jul 3, 2019

che-bot commented Jul 3, 2019

SkorikSergey commented Jul 3, 2019

sleshchenko commented Jun 21, 2019 •

edited

Loading