fix(workpace-plugin): Importing several private projects in a single devfile #959

sunix · 2021-01-05T17:46:26Z

What does this PR do?

Fix workspace start initial git cloning when multiple private projects are defined in the devfile.
Only ask for credentials once if cloning from the same host.
Adding a devfile.yaml for the workspace plugin (simplify review).
~~Fix jest import errors in Che~~
Include unit tests

What issues does this PR fix or reference?

How to test this PR?

create 2 private projects in Github with README files
start a workspace from the given devfile.
build the plugin with the predifined tasks
run che-theia with the predefined task

go to the dashboard and add the private projects to the devfile of the running workspace. For instance (adapt the location with the ones you have created):

  - name: private-1
    source:
      location: 'https://github.com/sunix/test-private-1'
      type: github
  - name: private-2
    source:
      location: 'https://github.com/sunix/test-private-2'
      type: github

SAVE the devfile DO NOT APPLY, DO NOT RESTART THE WORKSPACE!

back to che-theia, open the preview Url in a new tab, try to fill the username and password, observe the bug
stop the run task and close the preview tab, remove the content of /tmp/theiadev_projects from a terminal of che-theia-next-dev cotainer.
check the workspace devfile (dashboard) that the private projects are still there.
from the Github PR panel, checkout this PR
rebuild with the predifined task
rerun che-theia dev with the predifined task
open the Preview Url in a new tab.
fill your Github.com username and password, it should ask for it only once.
observe that the projects are cloned with the README files.

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Happy Path Channel

HAPPY_PATH_CHANNEL=next

che-bot · 2021-01-05T18:05:32Z

❌ E2E Happy path tests failed ❗

See Details

name	link
che-theia	docker.io/maxura/che-theia:959
che-theia-endpoint-runtime-binary	docker.io/maxura/che-theia-endpoint-runtime-binary:959

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

sunix · 2021-01-05T20:00:27Z

[crw-ci-test]

che-bot · 2021-01-05T20:05:43Z

❌ E2E Happy path tests failed ❗

See Details

name	link
che-theia	docker.io/maxura/che-theia:959
che-theia-endpoint-runtime-binary	docker.io/maxura/che-theia-endpoint-runtime-binary:959

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

benoitf · 2021-01-06T08:03:53Z

[crw-ci-test]

benoitf · 2021-01-06T08:04:19Z

FYI I switched happy path channel to next

che-bot · 2021-01-06T08:18:06Z

✅ E2E Happy path tests succeed 🎉

See Details

name	link
che-theia	docker.io/maxura/che-theia:959
che-theia-endpoint-runtime-binary	docker.io/maxura/che-theia-endpoint-runtime-binary:959

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

benoitf · 2021-01-06T08:24:13Z

plugins/workspace-plugin/devfile.yaml

@@ -0,0 +1,72 @@
+apiVersion: 1.0.0


I'm not sure it's a good practice to introduce devfiles in each directory ?

This a devfile to work on the workspace plugin, as this project lives in a subdirectory of the git repo, it looks good to have it there, next to the package.json, etc ...

but it's part of a yarn workspace so you should use top-level folder/definitions/devfile anyway to inherits from all workspace definition

ok but isn't it overkill when a dev just want to work on 1 plugin ?

It might be useful to have the devfile here, even just for reference purposes. It could be useful to point for new contributors in the future.

This devfile is for coding the workspace plugin without having to rebuild che-theia and all the other plugins. It doesn't make sense to have it elsewhere.

The maintenance issue is not related to the location of the devfiles. but how a devfile could be extended: so the devfile for the workspace plugin (that would be in the workspace plugin folder) could extend a more generic devfile for built-in plugins but adding its own specifics (for instance, custom commands, etc ...). Maybe it is in the scope of devfile 2.
Parent pom.xml files are usually there to share BoM. But without these sharing capabilities, we would have pom.xml files for each module that would be also hard to maintain. Anyway a module would have its own pom.xml.

Anyway, the devfile for the workspace plugin has to stay here because it would setup the right workspace for the workspace-plugin developer flow.

and then if the suggestion is to move that devfile to the devfiles directory I don't see how easier it would be to maintain. Or is the suggestion not to have devfile at all and let all the developers loosing time to setup an dev environment to work on each individual plugins?

No, the suggestion is to have a a single devfile for developing che-theia.

there is that devfile https://github.com/eclipse/che-theia/blob/master/devfiles/che-theia-all.devfile.yaml but it is doing way too many things when it is to work a a particular plugin.

benoitf · 2021-01-06T08:28:15Z

plugins/workspace-plugin/tests/file-uri.spec.ts

@@ -8,6 +8,8 @@
 * SPDX-License-Identifier: EPL-2.0
 ***********************************************************************/

+import 'jest';


looks odd to import jest ?

This was the only way I could find to get rid of the errors reported by the typescript plugin.

but jest is added on tsconfig types

no idea ...
maybe a bug in typescript plugin.

Do you want to remove these imports?

could you add

"types": ["reflect-metadata", "jest"],

in https://github.com/eclipse/che-theia/blob/master/plugins/workspace-plugin/tsconfig.json ?

like it's done there: https://github.com/eclipse/che-theia/blob/master/plugins/ports-plugin/tsconfig.json#L12-L14

I tried that already (see the M in tsconfig.ts in the screenshoft above). And just retried but it doesn't get rid of the error message. Maybe a bug in the plugin.

@benoitf I rebased and removed the imports

plugins/workspace-plugin/src/askpass-prompt-manager.ts

che-bot · 2021-01-08T12:00:22Z

✅ E2E Happy path tests succeed 🎉

See Details

name	link
che-theia	docker.io/maxura/che-theia:959
che-theia-endpoint-runtime-binary	docker.io/maxura/che-theia-endpoint-runtime-binary:959

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

…devfile Signed-off-by: Sun Tan <sutan@redhat.com>

Signed-off-by: Sun Tan <sutan@redhat.com>

che-bot · 2021-01-12T10:52:26Z

✅ E2E Happy path tests succeed 🎉

See Details

name	link
che-theia	docker.io/maxura/che-theia:959
che-theia-endpoint-runtime-binary	docker.io/maxura/che-theia-endpoint-runtime-binary:959

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

benoitf · 2021-01-12T11:04:26Z

@sunix there is still the devfile

sunix · 2021-01-12T13:56:05Z

@sunix there is still the devfile

I thought we were ok

to keep it for now
work on A single dogfooding devfile for che-theia che#18763
remove this devfile as soon as we would A single dogfooding devfile for che-theia che#18763 as well as other devfiles located in devfiles.

sunix requested review from azatsarynnyy, benoitf, ericwill, svor, tsmaeder, vinokurig and vitaliy-guliy as code owners January 5, 2021 17:46

benoitf reviewed Jan 6, 2021

View reviewed changes

benoitf reviewed Jan 7, 2021

View reviewed changes

plugins/workspace-plugin/src/askpass-prompt-manager.ts Outdated Show resolved Hide resolved

sunix requested a review from benoitf January 8, 2021 11:31

sunix added 4 commits January 12, 2021 08:52

fix(workpace-plugin): Importing several private projects in a single …

5c7575a

…devfile Signed-off-by: Sun Tan <sutan@redhat.com>

refactored, simplify sequence execution with Mutex

99e7489

Signed-off-by: Sun Tan <sutan@redhat.com>

revert imports jest

13c0a96

Signed-off-by: Sun Tan <sutan@redhat.com>

adding back mutex in lock file

004edb9

Signed-off-by: Sun Tan <sutan@redhat.com>

sunix force-pushed the fix-workspace-plugin-import-multiple-private-projects branch from 3bda7d6 to 004edb9 Compare January 12, 2021 10:14

removing import jest

7e31b9f

Signed-off-by: Sun Tan <sutan@redhat.com>

benoitf approved these changes Jan 12, 2021

View reviewed changes

sunix merged commit ff88e4e into eclipse-che:master Jan 12, 2021

sunix mentioned this pull request Jan 12, 2021

Importing several private projects in a single devfile fails eclipse-che/che#18494

Closed

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(workpace-plugin): Importing several private projects in a single devfile #959

fix(workpace-plugin): Importing several private projects in a single devfile #959

sunix commented Jan 5, 2021 •

edited

Loading

che-bot commented Jan 5, 2021

sunix commented Jan 5, 2021

che-bot commented Jan 5, 2021

benoitf commented Jan 6, 2021

benoitf commented Jan 6, 2021

che-bot commented Jan 6, 2021

benoitf Jan 6, 2021

sunix Jan 6, 2021

benoitf Jan 6, 2021

sunix Jan 6, 2021

ericwill Jan 6, 2021

sunix Jan 7, 2021

sunix Jan 7, 2021 •

edited

Loading

sunix Jan 7, 2021 •

edited

Loading

tsmaeder Jan 7, 2021

sunix Jan 7, 2021

benoitf Jan 6, 2021

sunix Jan 6, 2021 •

edited

Loading

benoitf Jan 6, 2021

sunix Jan 8, 2021

benoitf Jan 8, 2021

sunix Jan 11, 2021

sunix Jan 12, 2021

che-bot commented Jan 8, 2021

che-bot commented Jan 12, 2021

benoitf commented Jan 12, 2021

sunix commented Jan 12, 2021 •

edited

Loading

fix(workpace-plugin): Importing several private projects in a single devfile #959

fix(workpace-plugin): Importing several private projects in a single devfile #959

Conversation

sunix commented Jan 5, 2021 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

Happy Path Channel

che-bot commented Jan 5, 2021

sunix commented Jan 5, 2021

che-bot commented Jan 5, 2021

benoitf commented Jan 6, 2021

benoitf commented Jan 6, 2021

che-bot commented Jan 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunix Jan 7, 2021 • edited Loading

Choose a reason for hiding this comment

sunix Jan 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunix Jan 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

che-bot commented Jan 8, 2021

che-bot commented Jan 12, 2021

benoitf commented Jan 12, 2021

sunix commented Jan 12, 2021 • edited Loading

sunix commented Jan 5, 2021 •

edited

Loading

sunix Jan 7, 2021 •

edited

Loading

sunix Jan 7, 2021 •

edited

Loading

sunix Jan 6, 2021 •

edited

Loading

sunix commented Jan 12, 2021 •

edited

Loading