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

optimize(generic): support priority routing for HTTP generic call #739

Merged

Conversation

Marina-Sakai
Copy link
Contributor

@Marina-Sakai Marina-Sakai commented Dec 7, 2022

What type of PR is this?

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.

(Optional) Translate the PR title into Chinese.

(Optional) More detailed description for this PR(en: English/zh: Chinese).

en:
Kitex's URL routing does not align with hertz's one.
For example, if the routes are like this:

routes := []testRoute{
                {"/cmd/:tool", false},
                {"/cmd/:tool/:sub", false},
                {"/cmd/:tool/abc/:sub", false},
                {"/cmd/vet", false},
                {"/cmd/*filepath", false},
}

[The tree result in hertz]

/cmd/ - vet
          - : (tool) - / - abc - / - : (sub)
                         - : (sub)
          - * (filepath)

[The tree result in kitex]
/cmd/ - :tool
While hertz routes everything, kitex panics as the routes would conflict, except for the first route.
This MR aligns the kitex's routing way with the hertz's.
zh(optional):

Which issue(s) this PR fixes:

@codecov
Copy link

codecov bot commented Dec 7, 2022

Codecov Report

Base: 69.66% // Head: 69.75% // Increases project coverage by +0.08% 🎉

Coverage data is based on head (ee9de37) compared to base (75b446d).
Patch coverage: 97.35% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #739      +/-   ##
===========================================
+ Coverage    69.66%   69.75%   +0.08%     
===========================================
  Files          232      232              
  Lines        17318    17359      +41     
===========================================
+ Hits         12064    12108      +44     
+ Misses        4159     4156       -3     
  Partials      1095     1095              
Impacted Files Coverage Δ
pkg/generic/descriptor/tree.go 97.37% <97.23%> (-2.63%) ⬇️
pkg/generic/descriptor/router.go 75.00% <100.00%> (ø)
pkg/generic/thrift/parse.go 70.31% <100.00%> (+0.44%) ⬆️
pkg/remote/trans/nphttp2/grpc/controlbuf.go 81.23% <0.00%> (-1.11%) ⬇️
pkg/remote/trans/nphttp2/grpc/http2_client.go 73.59% <0.00%> (+0.15%) ⬆️
pkg/remote/trans/nphttp2/grpc/http2_server.go 71.22% <0.00%> (+0.31%) ⬆️
pkg/remote/trans/nphttp2/conn_pool.go 79.79% <0.00%> (+2.02%) ⬆️
...emote/trans/nphttp2/grpc/grpcframe/frame_reader.go 48.46% <0.00%> (+2.45%) ⬆️
pkg/remote/codec/default_codec.go 75.65% <0.00%> (+3.94%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Marina-Sakai Marina-Sakai changed the title WIP: optimize(generic): Support priority routing and unescape urls for HTTP generic call WIP: optimize(generic): Support priority routing for HTTP generic call Dec 8, 2022
@Marina-Sakai Marina-Sakai changed the title WIP: optimize(generic): Support priority routing for HTTP generic call optimize(generic): Support priority routing for HTTP generic call Dec 8, 2022
@Marina-Sakai Marina-Sakai force-pushed the optimize/http_generic_routing branch 6 times, most recently from f8da542 to db0fe7e Compare December 14, 2022 05:27
@Marina-Sakai Marina-Sakai requested review from a team as code owners December 14, 2022 05:27
@Marina-Sakai Marina-Sakai force-pushed the optimize/http_generic_routing branch 2 times, most recently from 48b4768 to e880960 Compare December 18, 2022 23:40
@Marina-Sakai Marina-Sakai changed the title optimize(generic): Support priority routing for HTTP generic call optimize(generic): support priority routing for HTTP generic call Dec 19, 2022
jayantxie
jayantxie previously approved these changes Dec 20, 2022
jayantxie
jayantxie previously approved these changes Dec 22, 2022
jayantxie
jayantxie previously approved these changes Jan 12, 2023
jayantxie
jayantxie previously approved these changes Feb 10, 2023
@Marina-Sakai Marina-Sakai force-pushed the optimize/http_generic_routing branch 2 times, most recently from c8c8124 to cc325a8 Compare February 10, 2023 10:08
YangruiEmma
YangruiEmma previously approved these changes Feb 10, 2023
@Marina-Sakai Marina-Sakai merged commit 7054d09 into cloudwego:develop Feb 13, 2023
@Marina-Sakai Marina-Sakai deleted the optimize/http_generic_routing branch February 13, 2023 04:24
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants