-
Notifications
You must be signed in to change notification settings - Fork 185
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
HTTP transport for Thrift #125
Comments
Is there a spec that describes the HTTP transport? |
Me neither, people were able to implement them in any language and do cross-test between each of the implementation so I think there should be a general idea. I agree that we need a spec about that. Apache Thrift had it supported since 0.1.x https://github.com/apache/thrift/blob/0.1.x/lib/cpp/src/transport/THttpClient.cpp |
Maybe we should submit an issue for that to the Apache thrift repo? |
apache/thrift repository seem don't accept issue from GitHub, Apache's Jira public signup is also disabled (https://infra.apache.org/jira-guidelines.html#who). Theoretically speaking, can HTTP Transport implemented with only current async fn make_transport(&self,addr:volo::net::Address) -> std::io::Result<(Self::ReadHalf,Self::WriteHalf)> And think that it's not the correct way of open a connection (over HTTP) since it need to be This may need to implement another Client beside existing multiplex and pingpong, the |
I'm not quite sure if we can implement HTTP transport using the current abstraction, but I think we can try it. For the server part, I think we need an underlying HTTP server which handle the HTTP protocol and exposes a read loop and write loop as the For the client part, this may be simpler than server, we can use a mature http client(such as reqwest) and use its own connection pool. I haven't thought about this carefully, so I'm not sure this will work. The biggest problems now are that:
|
hi, do we still need it? If yes, I want to give it a try |
Absolutely, that would be fantastic! We can figure out later on how we are going to merge the feature. Your offer of help is greatly appreciated. Thanks |
Hi, please kindly check the PR. Currently the implementation is using E.g. let url = "https://127.0.0.1/thrift-http-endpoint".parse::<hyper::Uri>().unwrap();
let url = volo::net::Address::from(url);
let client = volo_thrift::client::ClientBuilder::
new("dummy", dummys::MkDummyServiceGenericClient)
.make_codec(mk_codec)
.address(url)
.header("user-agent", "volo-http/1.0".parse().unwrap())
.build(); More detailed example can be accessed on https://github.com/ii64/volo_testing/blob/f796b7360e32493b450f907d27a048c936261b1d/the_example/src/bin/client_http_testing.rs |
@ii64 at a first glance, it seems great. Sorry for lack of activity for a long time. But im curious on your implementation, I will check your PR also. |
No worries, we are still doing RFC, you can check the discussion on linked PR |
Feature Request
Support for HTTP server/client transport wrapper for Thrift RPC.
Crates
volo-thrift
Motivation
Extending compatibility of Thrift implementation to support HTTP transport wrapper.
Current state of supported features compared with another Thrift library: apache/thrift/LANGUAGES.md
Proposal
Using
hyper
as Thrift HTTP transport wrapper, implementsvolo::net::dial::MakeTransport
.Alternatives
RFC
The text was updated successfully, but these errors were encountered: