Reconfigure Gson to parse more complex theia preference object #14442

vzhukovs · 2019-09-05T13:19:37Z

What does this PR do?

This changes proposal edits the rule which configure Gson parser to parse theia specific preferences. This mean that theia preferences not always contains simple structure string:string, but sometimes can be look like string:object. And could sometimes cause problem when workspace tried to start.

Signed-off-by: Vlad Zhukovskyi vzhukovs@redhat.com

What issues does this PR fix or reference?

#14436

Release Notes

N/A

Docs PR

N/A

skabashnyuk · 2019-09-05T13:47:53Z

@vzhukovskii what was the reason to save json in json?

...eclipse/che/workspace/infrastructure/kubernetes/provision/GitUserProfileProvisionerTest.java

...org/eclipse/che/workspace/infrastructure/kubernetes/provision/GitUserProfileProvisioner.java

vzhukovs · 2019-09-05T13:49:31Z

@skabashnyuk you mean json object of preferences in theia-user-preferences ?

skabashnyuk · 2019-09-05T13:51:04Z

yes

vzhukovs · 2019-09-05T13:53:42Z

That comes from https://github.com/eclipse/che-theia/blob/master/extensions/eclipse-che-theia-user-preferences/src/node/che-theia-preferences-synchronizer.ts#L87 and I think, that @mmorhun can clarify this.

vzhukovs · 2019-09-05T13:55:11Z

Looking from the code, that json object in theia-user-preferences is mirror of ~/.theia/settings.json, which is restoring on each workspace startup.

skabashnyuk · 2019-09-05T13:57:17Z

ok

...eclipse/che/workspace/infrastructure/kubernetes/provision/GitUserProfileProvisionerTest.java

amisevsk · 2019-09-05T14:15:10Z

...org/eclipse/che/workspace/infrastructure/kubernetes/provision/GitUserProfileProvisioner.java


-    return new Gson().fromJson(json, stringMapType);
+    return new Gson().fromJson(json, typeToken);


I'm not sure about how this is done; it looks like an implicit cast to Map<String, String>.

Yes, it's implicit, when we get parsed and implicitly casted map, we will look for two specific properties, that we're 100% sure that have string value. We can add additional checking for a string type, but it will reduce code readability. Wdyt?

It's up to you. It's not clear to me from reading the code that we can be 100% it's a string value, but I'm also not familiar with all the details here.

Is it possible for me to set my git.user.name to false?

Theoretically - yes, but it's hard to do, you need to update preferences by hand and escape the whole json object to place it as string value in user preferences. But when eclipse-che/che-theia#424 will be merged, these two properties will be configured through the UI and there will be an only option to setup them as string value.

I'll add a comment, describes the idea of this hack

Please make that any error in this class does not prevent workspace from starting.
To notify a user about an error you can provision a warning into k8s Environment, not sure if it's handled by Theia - if no - feel free to register an issue.

vitaliy-guliy

It looks I got updates this morning with nightly image.
Everything's working fine after restarting my minikube and workspace.

sleshchenko · 2019-09-06T07:53:26Z

It looks I got updates this morning with nightly image.

I'm afraid it's hardly possible to get PR changes with a nightly image

vitaliy-guliy · 2019-09-06T07:55:29Z

@sleshchenko ah, yes. I confused a bit :D

amisevsk

Please also remember to add the GitUserProfileProvisioner to the OpenShift environment provisioning (or add it in a follow-up PR).

vzhukovs · 2019-09-09T12:03:50Z

Please also remember to add the GitUserProfileProvisioner to the OpenShift environment provisioning (or add it in a follow-up PR).

Done. Checked on minishift, everything works now.

metlos

LGTM apart from the fact that the OpenShift support infra registration should maybe warrant its own PR.

metlos · 2019-09-09T12:39:49Z

...java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftEnvironmentProvisioner.java

@@ -86,7 +88,8 @@ public OpenShiftEnvironmentProvisioner(
      ProxySettingsProvisioner proxySettingsProvisioner,
      ServiceAccountProvisioner serviceAccountProvisioner,
      CertificateProvisioner certificateProvisioner,
-      VcsSshKeysProvisioner vcsSshKeysProvisioner) {
+      VcsSshKeysProvisioner vcsSshKeysProvisioner,
+      GitUserProfileProvisioner gitUserProfileProvisioner) {


These changes seem unrelated to the bug this PR is fixing. They seem add support for the git prefs provisioning to Openshift which IMHO is a whole another issue.

Good, I'll provide dedicate PR for this registration. @amisevsk fyi.

@metlos @amisevsk #14488

che-bot · 2019-09-09T15:51:33Z

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

build details
"che-server" docker image: maxura/che-server:14442
test artifacts
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.
Use command "crw-ci-test" to rerun the test.

che-bot · 2019-09-09T16:04:48Z

E2E tests of Eclipse Che Multiuser on OCP has been successful:

Build details
Test report
docker image: eclipseche/che-server:14442

vzhukovs · 2019-09-10T08:14:52Z

crw-ci-test

che-bot · 2019-09-10T08:35:07Z

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

build details
"che-server" docker image: maxura/che-server:14442
test artifacts
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.
Use command "crw-ci-test" to rerun the test.

dmytro-ndp · 2019-09-10T08:44:19Z

@vkuznyetsov: you need to take latest changes of Happy path tests from master branch to fix crw-ci-e2e-happy-path-tests PR check build.

che-bot · 2019-09-10T08:59:30Z

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

build details
"che-server" docker image: maxura/che-server:14442

che-bot · 2019-09-10T09:29:17Z

E2E tests of Eclipse Che Multiuser on OCP has been successful:

Build details
Test report
docker image: eclipseche/che-server:14442

Signed-off-by: Vlad Zhukovskyi <vzhukovs@redhat.com>

che-bot · 2019-09-10T12:48:33Z

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

build details
"che-server" docker image: maxura/che-server:14442

che-bot · 2019-09-10T13:15:53Z

E2E tests of Eclipse Che Multiuser on OCP has failed:

Build details
Test report
docker image: eclipseche/che-server:14442
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.
Use command "ci-test" to rerun the test.

vzhukovs added kind/bug Outline of a bug - must adhere to the bug report template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. team/ide labels Sep 5, 2019

vzhukovs added this to the 7.2.0 milestone Sep 5, 2019

vzhukovs requested review from amisevsk, l0rd, metlos, nickboldt, rhopp and sleshchenko as code owners September 5, 2019 13:19

vzhukovs self-assigned this Sep 5, 2019

vzhukovs requested review from azatsarynnyy and vitaliy-guliy September 5, 2019 13:19

amisevsk reviewed Sep 5, 2019

View reviewed changes

...eclipse/che/workspace/infrastructure/kubernetes/provision/GitUserProfileProvisionerTest.java Outdated Show resolved Hide resolved

...org/eclipse/che/workspace/infrastructure/kubernetes/provision/GitUserProfileProvisioner.java Outdated Show resolved Hide resolved

amisevsk reviewed Sep 5, 2019

View reviewed changes

vzhukovs mentioned this pull request Sep 5, 2019

Provision git configuration into each container #14402

Merged

vitaliy-guliy approved these changes Sep 6, 2019

View reviewed changes

amisevsk reviewed Sep 6, 2019

View reviewed changes

metlos reviewed Sep 9, 2019

View reviewed changes

vzhukovs force-pushed the che#14436 branch from 3a423fa to da168ee Compare September 9, 2019 12:58

metlos approved these changes Sep 9, 2019

View reviewed changes

eclipse-che deleted a comment from che-bot Sep 10, 2019

Reconfigure Gson to parse more complex theia preference object

a322997

Signed-off-by: Vlad Zhukovskyi <vzhukovs@redhat.com>

vzhukovs force-pushed the che#14436 branch from 5f0027f to a322997 Compare September 10, 2019 12:38

vzhukovs merged commit 8c6ba05 into master Sep 10, 2019

vzhukovs deleted the che#14436 branch September 10, 2019 13:17

che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Sep 10, 2019

vzhukovs mentioned this pull request Sep 10, 2019

Add missing registration for git preferences provisioner #14488

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reconfigure Gson to parse more complex theia preference object #14442

Reconfigure Gson to parse more complex theia preference object #14442

vzhukovs commented Sep 5, 2019

skabashnyuk commented Sep 5, 2019

vzhukovs commented Sep 5, 2019

skabashnyuk commented Sep 5, 2019

vzhukovs commented Sep 5, 2019

vzhukovs commented Sep 5, 2019

skabashnyuk commented Sep 5, 2019

amisevsk Sep 5, 2019

vzhukovs Sep 5, 2019

amisevsk Sep 5, 2019

vzhukovs Sep 5, 2019

vzhukovs Sep 5, 2019

sleshchenko Sep 5, 2019

vitaliy-guliy left a comment •

edited

Loading

sleshchenko commented Sep 6, 2019

vitaliy-guliy commented Sep 6, 2019

amisevsk left a comment

vzhukovs commented Sep 9, 2019

metlos left a comment

metlos Sep 9, 2019

vzhukovs Sep 9, 2019

vzhukovs Sep 9, 2019

amisevsk Sep 9, 2019

che-bot commented Sep 9, 2019

che-bot commented Sep 9, 2019

vzhukovs commented Sep 10, 2019

che-bot commented Sep 10, 2019

dmytro-ndp commented Sep 10, 2019

che-bot commented Sep 10, 2019

che-bot commented Sep 10, 2019

che-bot commented Sep 10, 2019

che-bot commented Sep 10, 2019


		return new Gson().fromJson(json, stringMapType);
		return new Gson().fromJson(json, typeToken);

Reconfigure Gson to parse more complex theia preference object #14442

Reconfigure Gson to parse more complex theia preference object #14442

Conversation

vzhukovs commented Sep 5, 2019

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

skabashnyuk commented Sep 5, 2019

vzhukovs commented Sep 5, 2019

skabashnyuk commented Sep 5, 2019

vzhukovs commented Sep 5, 2019

vzhukovs commented Sep 5, 2019

skabashnyuk commented Sep 5, 2019

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

vitaliy-guliy left a comment • edited Loading

Choose a reason for hiding this comment

sleshchenko commented Sep 6, 2019

vitaliy-guliy commented Sep 6, 2019

amisevsk left a comment

Choose a reason for hiding this comment

vzhukovs commented Sep 9, 2019

metlos left a 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 Sep 9, 2019

che-bot commented Sep 9, 2019

vzhukovs commented Sep 10, 2019

che-bot commented Sep 10, 2019

dmytro-ndp commented Sep 10, 2019

che-bot commented Sep 10, 2019

che-bot commented Sep 10, 2019

che-bot commented Sep 10, 2019

che-bot commented Sep 10, 2019

vitaliy-guliy left a comment •

edited

Loading