fix: Apply hack to allow to use ctrl-p on terminal #1201

mtsmfm · 2021-08-22T15:23:58Z

What does this PR do?

Currently, ctrl-p on terminal doesn't work because ctrl-p ctrl-q is docker's default detach keys and there's no way to customize it with k8s api.

kubernetes/kubectl#1011

Screenshot/screencast of this PR

before

after

What issues does this PR fix or reference?

How to test this PR?

The test platform: docker-desktop
Installation method: chectl
steps to reproduce
- open terminal
- runecho hi
- type ctrl-p

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

che-bot · 2021-08-22T15:24:01Z

Can one of the admins verify this patch?

codecov · 2021-08-22T16:50:38Z

Codecov Report

Merging #1201 (2ba01cf) into main (c299f59) will decrease coverage by 0.17%.
The diff coverage is 18.88%.

@@            Coverage Diff             @@
##             main    #1201      +/-   ##
==========================================
- Coverage   32.78%   32.60%   -0.18%     
==========================================
  Files         290      295       +5     
  Lines        9885     9997     +112     
  Branches     1457     1536      +79     
==========================================
+ Hits         3241     3260      +19     
+ Misses       6641     6628      -13     
- Partials        3      109     +106

Impacted Files	Coverage Δ
...theia-about/src/browser/about-che-theia-dialog.tsx	`0.00% <0.00%> (ø)`
...credentials/src/browser/che-credentials-service.ts	`0.00% <0.00%> (ø)`
...entials/src/browser/credentials-frontend-module.ts	`0.00% <0.00%> (ø)`
...eia-credentials/src/common/credentials-protocol.ts	`0.00% <0.00%> (ø)`
...eia-credentials/src/node/che-credentials-server.ts	`0.00% <0.00%> (ø)`
...s/src/node/che-theia-credentials-backend-module.ts	`0.00% <0.00%> (ø)`
...rowser/src/browser/che-mini-browser-environment.ts	`0.00% <0.00%> (ø)`
...he-server/src/node/che-server-http-service-impl.ts	`0.00% <0.00%> (ø)`
...-che-server/src/node/che-server-remote-api-impl.ts	`38.88% <0.00%> (ø)`
...browser/contribution/exec-terminal-contribution.ts	`0.00% <0.00%> (ø)`
... and 76 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 2dc9aed...2ba01cf. Read the comment docs.

azatsarynnyy · 2021-08-30T17:12:00Z

[crw-ci-test]

che-bot · 2021-08-30T17:34:44Z

❌ E2E Happy path tests failed ❗

See Details

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

Test product:

tested with Eclipse Che on K8S (minikube v1.22.0)
E2E test scenario
E2E test pipeline source code

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.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

vitaliy-guliy · 2021-09-02T11:51:29Z

[crw-ci-test]

che-bot · 2021-09-02T12:15:24Z

❌ E2E Happy path tests failed ❗

See Details

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

Test product:

tested with Eclipse Che on K8S (minikube v1.22.0)
E2E test scenario
E2E test pipeline source code

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.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

mtsmfm · 2021-09-04T11:39:35Z

@azatsarynnyy @vitaliy-guliy What can I do to move this PR forward?

I can't see any E2E results https://main-jenkins-csb-crwqe.apps.ocp4.prod.psi.redhat.com/blue/organizations/jenkins/Che%2Fe2e%2Fminikube%2Fpr-check%2Fche-theia-pr-check/detail/che-theia-pr-check/445

#1201 (comment)

vitaliy-guliy · 2021-09-07T14:02:45Z

@mtsmfm please update your branch the first. che-theia/main has important updates, which allows happy path tests to be passed.

mtsmfm · 2021-09-07T14:23:46Z

@vitaliy-guliy Rebased and force pushed.

azatsarynnyy · 2021-09-07T16:58:34Z

[crw-ci-test]

che-bot · 2021-09-08T04:37:45Z

❌ E2E Happy path tests failed ❗

See Details

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

Test product:

tested with Eclipse Che on K8S (minikube v1.22.0)
E2E test scenario
E2E test pipeline source code

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.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

azatsarynnyy · 2021-09-08T09:48:58Z

[crw-ci-test]

che-bot · 2021-09-08T10:12:37Z

❌ E2E Happy path tests failed ❗

See Details

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

Test product:

tested with Eclipse Che on K8S (minikube v1.22.0)
E2E test scenario
E2E test pipeline source code

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.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

azatsarynnyy · 2021-09-08T16:49:55Z

For some reason, the images for running HappyPath tests aren't built.
@dmytro-ndp could you please take a look?

dmytro-ndp · 2021-09-09T12:29:59Z

@azatsarynnyy: you need to add comment with [crw-ci-test --rebuild] to build theia images from PR

dmytro-ndp · 2021-09-09T12:31:40Z

[crw-ci-test --rebuild]

che-bot · 2021-09-09T13:12:55Z

✅ E2E Happy path tests succeed 🎉

See Details

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

Test product:

tested with Eclipse Che on K8S (minikube v1.22.0)
E2E test scenario
E2E test pipeline source code

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.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

azatsarynnyy

I'm not able to test how it solves the problem on docker-desktop. If someone can - please do.
At least, I've checked that the patch doesn't break anything by testing it on DevSandbox.

mtsmfm · 2021-09-27T12:31:27Z

I've found \u0000 is handle by zsh...
I'll change status to draft.

mtsmfm requested review from azatsarynnyy, benoitf, RomanNikitenko and vzhukovs as code owners August 22, 2021 15:23

mtsmfm force-pushed the ctrl-p branch from 203780c to 1ce1030 Compare August 22, 2021 15:25

mtsmfm changed the title ~~Apply hack to allow to use ctrl-p on terminal~~ fix: Apply hack to allow to use ctrl-p on terminal Aug 22, 2021

mtsmfm force-pushed the ctrl-p branch from 1ce1030 to dbdebcd Compare August 22, 2021 16:37

fix: Apply hack to allow to use ctrl-p on terminal

2ba01cf

mtsmfm force-pushed the ctrl-p branch from dbdebcd to 2ba01cf Compare September 7, 2021 14:22

azatsarynnyy approved these changes Sep 9, 2021

View reviewed changes

mtsmfm marked this pull request as draft September 27, 2021 12:31

azatsarynnyy self-requested a review October 19, 2021 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Apply hack to allow to use ctrl-p on terminal #1201

fix: Apply hack to allow to use ctrl-p on terminal #1201

mtsmfm commented Aug 22, 2021 •

edited

Loading

che-bot commented Aug 22, 2021

codecov bot commented Aug 22, 2021 •

edited

Loading

azatsarynnyy commented Aug 30, 2021

che-bot commented Aug 30, 2021

vitaliy-guliy commented Sep 2, 2021

che-bot commented Sep 2, 2021

mtsmfm commented Sep 4, 2021 •

edited

Loading

vitaliy-guliy commented Sep 7, 2021

mtsmfm commented Sep 7, 2021

azatsarynnyy commented Sep 7, 2021

che-bot commented Sep 8, 2021

azatsarynnyy commented Sep 8, 2021

che-bot commented Sep 8, 2021

azatsarynnyy commented Sep 8, 2021

dmytro-ndp commented Sep 9, 2021

dmytro-ndp commented Sep 9, 2021

che-bot commented Sep 9, 2021

azatsarynnyy left a comment

mtsmfm commented Sep 27, 2021

fix: Apply hack to allow to use ctrl-p on terminal #1201

Are you sure you want to change the base?

fix: Apply hack to allow to use ctrl-p on terminal #1201

Conversation

mtsmfm commented Aug 22, 2021 • edited Loading

What does this PR do?

Screenshot/screencast of this PR

before

after

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

Happy Path Channel

che-bot commented Aug 22, 2021

codecov bot commented Aug 22, 2021 • edited Loading

Codecov Report

azatsarynnyy commented Aug 30, 2021

che-bot commented Aug 30, 2021

vitaliy-guliy commented Sep 2, 2021

che-bot commented Sep 2, 2021

mtsmfm commented Sep 4, 2021 • edited Loading

vitaliy-guliy commented Sep 7, 2021

mtsmfm commented Sep 7, 2021

azatsarynnyy commented Sep 7, 2021

che-bot commented Sep 8, 2021

azatsarynnyy commented Sep 8, 2021

che-bot commented Sep 8, 2021

azatsarynnyy commented Sep 8, 2021

dmytro-ndp commented Sep 9, 2021

dmytro-ndp commented Sep 9, 2021

che-bot commented Sep 9, 2021

azatsarynnyy left a comment

Choose a reason for hiding this comment

mtsmfm commented Sep 27, 2021

mtsmfm commented Aug 22, 2021 •

edited

Loading

codecov bot commented Aug 22, 2021 •

edited

Loading

mtsmfm commented Sep 4, 2021 •

edited

Loading