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.

Sign up for GitHub

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

Jump to bottom

Remove http2 #3713

Merged
merged 12 commits into from
Feb 14, 2022
Merged

Remove http2 #3713

merged 12 commits into from
Feb 14, 2022

Conversation

jackwener
Copy link
Contributor

@jackwener jackwener commented Jan 13, 2022 •
edited
Loading

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number: #3149

Description:

remove http2

How do you solve it?

Special notes for your reviewer, ex. impact of this fix, design document, etc:

Checklist:

Tests:

  • Unit test(positive and negative cases)
  • Function test
  • Performance test
  • N/A

Affects:

  • Documentation affected (Please add the label if documentation needs to be modified.)
  • Incompatibility (If it breaks the compatibility, please describe it and add the label.）
  • If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
  • Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to be reflected in release notes and how to describe:

ex. Fixed the bug .....

@jackwener jackwener changed the title remove h2 Remove http2 Jan 13, 2022
@jackwener jackwener added ready for review ready-for-testing PR: ready for the CI test labels Jan 13, 2022
@Shylock-Hg
Copy link
Contributor

Why remove it?

@darionyaphet
Copy link
Contributor

StorageHttpPropertyHandlerTest also include this flag

@jackwener jackwener force-pushed the http2 branch 3 times, most recently from 9c7af28 to 8ef01e1 Compare January 13, 2022 07:35
@Sophie-Xie Sophie-Xie added the doc affected PR: improvements or additions to documentation label Jan 13, 2022
@Sophie-Xie
Copy link
Contributor

Sophie-Xie commented Jan 13, 2022 •
edited
Loading

Issue 3149 is not just remove http2, and hope to support use nGQL to show service's http_port.

@jackwener
Copy link
Contributor Author

Issue 3149 is not just remove http2, and hope to support use nGQL to show service's http_port.

Finish in two PR because they is different things.

@jackwener jackwener linked an issue Jan 13, 2022 that may be closed by this pull request
support use nGQL to show service's http_port and disable http2_port #3149
Closed
critical27
critical27 previously approved these changes Jan 24, 2022
View reviewed changes
CPWstatic
CPWstatic previously approved these changes Jan 25, 2022
View reviewed changes
@jackwener jackwener requested a review from a team as a code owner January 25, 2022 09:15
@jackwener jackwener dismissed stale reviews from CPWstatic and critical27 via b62f861 February 8, 2022 02:09
yixinglu
yixinglu approved these changes Feb 8, 2022
View reviewed changes
Copy link
Contributor

@yixinglu yixinglu left a comment

Choose a reason for hiding this comment

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

LGTM if having made the decision.

@yixinglu yixinglu added the incompatible PR: incompatible with the recently released version label Feb 8, 2022
@jackwener
Copy link
Contributor Author

When the old version upgrade to the new, the conf file will be old version.
It's OK, because the redundant h2 config will be ignored.

@yixinglu yixinglu merged commit 5a4be98 into vesoft-inc:master Feb 14, 2022
@jackwener jackwener deleted the http2 branch February 14, 2022 08:12
liwenhui-soul pushed a commit to liwenhui-soul/nebula that referenced this pull request Feb 15, 2022
@jackwener @liwenhui-soul
remove h2 (vesoft-inc#3713)
aa3912a
@jamieliu1023 jamieliu1023 mentioned this pull request Feb 19, 2022
Closed
@cooper-lzy cooper-lzy mentioned this pull request Mar 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperYoko SuperYoko SuperYoko left review comments

@nevermore3 nevermore3 nevermore3 approved these changes

@yixinglu yixinglu yixinglu approved these changes

@critical27 critical27 critical27 left review comments

@CPWstatic CPWstatic CPWstatic left review comments

@jievince jievince Awaiting requested review from jievince

Assignees
No one assigned
Labels
doc affected PR: improvements or additions to documentation incompatible PR: incompatible with the recently released version ready for review ready-for-testing PR: ready for the CI test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

support use nGQL to show service's http_port and disable http2_port
9 participants
@jackwener @Shylock-Hg @darionyaphet @Sophie-Xie @yixinglu @nevermore3 @CPWstatic @critical27 @SuperYoko