Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Replace WorkspaceService by DevfileService #1055

Merged
merged 1 commit into from
Mar 27, 2021

Conversation

RomanNikitenko
Copy link
Member

@RomanNikitenko RomanNikitenko commented Mar 26, 2021

Signed-off-by: Roman Nikitenko rnikiten@redhat.com

What does this PR do?

Replace WorkspaceService by DevfileService to have multi-root related logic working for DevWorkspace engine.
Also please see the comments

Screenshot/screencast of this PR

What issues does this PR fix or reference?

Related to:

How to test this PR?

devfile_V2 use case -DevWorkspace: a workspace should start in Multi-root mode by default.

devfile_V1 use case:

  1. Start a workspace and check that the workspace has started in Multi-root mode by default.
  2. Add the following to your devfile:
attributes:
  multiRoot: 'off'

  1. Start the workspace and check that the Multi-root mode is off.

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=stable

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
@RomanNikitenko
Copy link
Member Author

I tried to test the changes for DevWorkspace, but faced with the error:

error: error validating "flattened_theia-next.yaml": error validating data: ValidationError(DevWorkspace.metadata): unknown field "attributes" in io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta; if you choose to ignore these errors, turn validation off with --validate=false

Does metadata supports attributes?

My flattened yaml file starts like:

kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: theia
  attributes:
    multiRoot: 'off'
spec:
  started: true
  template:
    projects:

@benoitf could you take a look?

@codecov-io
Copy link

Codecov Report

Merging #1055 (46a079c) into master (eeb4526) will increase coverage by 0.74%.
The diff coverage is 82.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1055      +/-   ##
==========================================
+ Coverage   29.45%   30.20%   +0.74%     
==========================================
  Files         277      277              
  Lines        9336     9366      +30     
  Branches     1380     1390      +10     
==========================================
+ Hits         2750     2829      +79     
+ Misses       6487     6439      -48     
+ Partials       99       98       -1     
Impacted Files Coverage Δ
...che-theia-remote-api/src/common/devfile-service.ts 0.00% <ø> (ø)
...e-theia-workspace/src/node/che-workspace-server.ts 0.00% <0.00%> (ø)
...workspace-plugin/src/workspace-projects-manager.ts 0.00% <0.00%> (ø)
...che-server/src/node/che-server-k8s-service-impl.ts 39.13% <50.00%> (+39.13%) ⬆️
...server/src/node/che-server-devfile-service-impl.ts 86.56% <95.00%> (+4.79%) ⬆️
...s-provider-extension/src/browser/prefs-provider.ts 95.74% <100.00%> (+3.63%) ⬆️
...mote-impl-k8s/src/node/k8s-devfile-service-impl.ts 84.61% <100.00%> (+0.61%) ⬆️
...rver/src/node/che-server-workspace-service-impl.ts 37.50% <0.00%> (+37.50%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6429b29...46a079c. Read the comment docs.

@benoitf
Copy link
Contributor

benoitf commented Mar 26, 2021

@RomanNikitenko for now it's not but your PR is ok
(DevFile supports it but not the DevWorkspaceTemplate but it'll be changed to global attributes, etc)

As long as we're using DevfileService to get the devfile it's better

@benoitf
Copy link
Contributor

benoitf commented Mar 26, 2021

cf devfile/api#239

@che-bot
Copy link
Contributor

che-bot commented Mar 26, 2021

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia quay.io/crw_pr/che-theia:1055
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1055

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.

@RomanNikitenko
Copy link
Member Author

@RomanNikitenko for now it's not but your PR is ok
(DevFile supports it but not the DevWorkspaceTemplate but it'll be changed to global attributes, etc)

As long as we're using DevfileService to get the devfile it's better

I see, thanks!

@RomanNikitenko
Copy link
Member Author

[crw-ci-test]

@che-bot
Copy link
Contributor

che-bot commented Mar 27, 2021

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia quay.io/crw_pr/che-theia:1055
che-theia-endpoint-runtime-binary quay.io/crw_pr/che-theia-endpoint-runtime-binary:1055

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.

@RomanNikitenko RomanNikitenko marked this pull request as ready for review March 27, 2021 07:45
@benoitf benoitf merged commit 4dfcd66 into eclipse-che:master Mar 27, 2021
@che-bot che-bot added this to the 7.29 milestone Mar 27, 2021
@RomanNikitenko RomanNikitenko deleted the replaceWorkspaceService branch March 27, 2021 14:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants