Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sidecar based maven and gradle Java stacks #12898

Merged
merged 8 commits into from
Mar 29, 2019

Conversation

l0rd
Copy link
Contributor

@l0rd l0rd commented Mar 15, 2019

What does this PR do?

  • Add stack java-maven
  • Add stack java-gradle
  • Modify samples to work with the new stacks
  • Add a new spring-boot sample using this spring guide repo

Some choices made to build the new stacks:

  • che-theia:next is used instead of latest
  • I am using the default DockerHub images as runtime with JDK11 on Debian (i.e. maven and gradle)
  • The runtime images tag is specified (don't use latest). I wanted to use sha256 tags (more reliable) but stacks API complained about that. Will create an issue.
  • Memory:
    • che-theia: 512MB
    • maven runtime: 512MB
    • jdt.ls: 1GB
  • The ${current.project.path} placeholder for commands doesn't work on Theia and I have replaced it with ${CHE_PROJECTS_ROOT}/<sample-name>

Current Limitations

This PR is related to che-samples/console-java-simple#7

@rhopp with that PR the good old che 6 java stack is gone. I suspect this will have some side effects on QE...

@benoitf
Copy link
Contributor

benoitf commented Mar 15, 2019

che-theia:next no ?

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Please consider applying my proposals just to have tags in the same style

ide/che-core-ide-stacks/src/main/resources/stacks.json Outdated Show resolved Hide resolved
ide/che-core-ide-stacks/src/main/resources/stacks.json Outdated Show resolved Hide resolved
ide/che-core-ide-stacks/src/main/resources/stacks.json Outdated Show resolved Hide resolved
ide/che-core-ide-templates/src/main/resources/samples.json Outdated Show resolved Hide resolved
@ibuziuk
Copy link
Member

ibuziuk commented Mar 15, 2019

I guess the plan is also to replace it for rh-che, right ?

@garagatyi
Copy link

Trying to run java extension in a sidecar on current master and get next error:
image
Does it work for somebody else?

@slemeur
Copy link
Contributor

slemeur commented Mar 15, 2019

Nice.

I also saw a companion PR to update an existing sample: https://github.com/che-samples/console-java-simple
I think that can potentially break the sample in older version of Che?

@garagatyi
Copy link

I restarted workspace with Java plugin several times, changed default RAM, but still no java assistance. Is it known bug? Do we want to replace stack with such a state?

@l0rd
Copy link
Contributor Author

l0rd commented Mar 15, 2019

Nice.

I also saw a companion PR to update an existing sample: https://github.com/che-samples/console-java-simple
I think that can potentially break the sample in older version of Che?

@slemeur it may break versions of Che that use a version of JDK older than JDK 8 (2014). Is that an issue?

I restarted workspace with Java plugin several times, changed default RAM, but still no java assistance. Is it known bug? Do we want to replace stack with such a state?

@garagatyi agree that's annoying. I couldn't make it work neither. That's not related to JDT LS but to any vs code extension running in a sidecar. I have been pointed to eclipse-theia/theia#3970. I was hoping that it could be fixed before this PR got merged because anyway it's critical. @benoitf @tolusha any comments about that?

That said if that cannot be fixed by the end of the sprint I am ok to not replace this stack but just create a distinct one (i.e. java-maven). But I hope we can proceed we the replacement and start moving forward with Che 7 (beta i.e. not perfect) stacks only.

By the way this make me think that even if we fix the JDT LS loading problem we still have the following limitations (added that to the issue description):

  1. Java Debugging is not working (we need multiple vscode ext on the same sidecar)
  2. JDT LS files are not persisted in a volume (that's related to )

@garagatyi
Copy link

@l0rd we will be able to create plugin that includes both java and java debugger soon #12903

@benoitf
Copy link
Contributor

benoitf commented Mar 15, 2019

@l0rd about eclipse-theia/theia#3970 / plug-ins reload I'm working on it since the last couple of days and made several fixes. Another fix is pending eclipse-theia/theia#4583
Hopefully it may be the last one.

@l0rd
Copy link
Contributor Author

l0rd commented Mar 15, 2019

@garagatyi @benoitf cool thank you for the update on those issues. So it looks like the situation is under control. @benoitf it would be great if you could test this java stack with your patch (see if JDT LS work fine) or if you could point me to a che-theia to use to test.

My main concern with this PR is QE team anyway. @rhopp I would like to merge it as it is to turn the page and don't think about Che 6 java stack anymore :-) But if that has a huge impact on your test I can keep (maybe hiding it) the existing default java stack.

sleshchenko and others added 4 commits March 15, 2019 16:21
Co-Authored-By: l0rd <mario.loriedo@gmail.com>
Co-Authored-By: l0rd <mario.loriedo@gmail.com>
Co-Authored-By: l0rd <mario.loriedo@gmail.com>
Co-Authored-By: l0rd <mario.loriedo@gmail.com>
@l0rd l0rd requested a review from tolusha March 15, 2019 15:22
@benoitf
Copy link
Contributor

benoitf commented Mar 15, 2019

@l0rd I had issues on 'plug-in reload' with vscode-java extension and fixed it upstream redhat-developer/vscode-java#832

@slemeur
Copy link
Contributor

slemeur commented Mar 15, 2019

@slemeur it may break versions of Che that use a version of JDK older than JDK 8 (2014). Is that an issue?

No, it's not 👍 Thanks for clarifying!

@SkorikSergey
Copy link
Contributor

ci-test

@che-bot
Copy link
Contributor

che-bot commented Mar 18, 2019

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

@SkorikSergey
Copy link
Contributor

Selenium tests execution on Eclipse Che Multiuser on OCP (https://ci.codenvycorp.com/job/che-pullrequests-test-ocp/1634//Selenium_20tests_20report/) showed regression in selenium tests from dashboard package that use Java stack.

Failed selenium tests will be updated by #12913 after this PR is merged.

Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
@l0rd
Copy link
Contributor Author

l0rd commented Mar 20, 2019

For the record I will update this PR to keep the old GWT based java-default stack. I updating it because JDT LS is never loaded when using this stack hence there is no Java support yet (it should be related to #12904).

@l0rd l0rd changed the title Replace Default Java stack with a Che 7 based one Add sidecar based maven and gradle Java stacks Mar 28, 2019
@l0rd
Copy link
Contributor Author

l0rd commented Mar 28, 2019

@SkorikSergey I have updated the PR to keep the old java-default stack. We cannot replace the old stack until we have proper debugging support with JDT LS running in a sidecar (#12446) and I would like to merge this PR in the meantime. Is it ok for you to merge?

@SkorikSergey
Copy link
Contributor

@l0rd It's Ok for me. There will need to make small changes to selenium tests. I will do it after merging.

@l0rd l0rd merged commit 203d4d5 into eclipse-che:master Mar 29, 2019
@l0rd l0rd deleted the che7javastacks branch March 29, 2019 10:34
@sunix
Copy link
Contributor

sunix commented Apr 2, 2019

@l0rd I am coming a bit late on this one:
Why don't we use the same container to host VSCode Java and java/graddle/maven command line tool ?
It makes sense that they all share the same versions. We would avoid duplication IMHO and would consume less memory.

@l0rd
Copy link
Contributor Author

l0rd commented Apr 3, 2019

@sunix there is a thread in che-dev (Custom builds and side cars) where we discuss that. You may want to have a look at it and participate to the conversation?

@che-bot
Copy link
Contributor

che-bot commented Apr 4, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants