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

Disable http2 by default for java8 #16964

Merged
merged 2 commits into from
May 18, 2020
Merged

Conversation

skabashnyuk
Copy link
Contributor

What does this PR do?

Disable http2 by default for java8. OkHttp wrongly detects JDK8u251 and higher as JDK9 which enables Http2 unsupported for JDK8.

What issues does this PR fix or reference?

Workaround for #16944

…nd higher as JDK9 which enables Http2 unsupported for JDK8.

Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>
@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels May 18, 2020
@che-bot
Copy link
Contributor

che-bot commented May 18, 2020

❌ E2E Happy path tests failed ❗

See Details

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

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

@skabashnyuk
Copy link
Contributor Author

crw-ci-test

@skabashnyuk
Copy link
Contributor Author

ci-test

@che-bot
Copy link
Contributor

che-bot commented May 18, 2020

✅ E2E Happy path tests succeed 🎉

See Details

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

metlos
metlos previously approved these changes May 18, 2020
Copy link
Contributor

@metlos metlos left a comment

Choose a reason for hiding this comment

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

I think this is OK. Setting this through a system property actually makes it non-overridable using env var, which I think is a good thing, because it is not supported on Java 8 anyway.

Please disregard this in favor of my other review...

@metlos metlos self-requested a review May 18, 2020 14:00
Copy link
Contributor

@metlos metlos left a comment

Choose a reason for hiding this comment

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

I'm not sure we should keep the possibility to make the http2 transport configurable by env var on java8...

@metlos metlos dismissed their stale review May 18, 2020 14:05

I misread the code.

…8u251 and higher as JDK9 which enables Http2 unsupported for JDK8.

Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented May 18, 2020

✅ E2E Happy path tests succeed 🎉

See Details

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

@skabashnyuk skabashnyuk added this to the 7.14 milestone May 18, 2020
@skabashnyuk skabashnyuk merged commit fb063a9 into eclipse-che:master May 18, 2020
@skabashnyuk skabashnyuk deleted the che16944 branch May 18, 2020 18:25
@che-bot 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 May 18, 2020
mkuznyetsov pushed a commit that referenced this pull request May 19, 2020
* Disable http2 by default for java8. OkHttp wrongly detects JDK8u251 and higher as JDK9 which enables Http2 unsupported for JDK8.

Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants