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 HTTP client generator #445

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

vanjabucic
Copy link

First of all, thank you for this well written Thrift library.

Want: to use Elixir with an existing Thrift web service written in Java.

History:
I tried to modify the reference Erlang Apache Thrift implementation for use in a web client.
It worked for me but not clean enough for PR merge (due to fundamental structural issues in the Erlang code). It was very ugly.
Then I found this project with clearly delineated functionalities and modules.
It enabled me to quickly create a working Thrift web service client generator for our needs (included in this PR)

Discussion:
You thoughts on having support for HTTP clients as part of this project?

@fishcakez
Copy link
Member

Hi @vanjabucic, we are significantly changing how we generate clients/servers so that code generation is decoupled from client/server implementation. The start of the work is in #441. Ideally a HTTP client would not do any generation once that PR is landed, instead we generate per protocol (e.g. BinaryProtocol), and then dispatch to generic client transport implementation (e.g. framed, or http). If we do include a http client I would prefer it to be https://github.com/ericmj/xhttp, though don't know if its ready for a release yet. Of course with other diff, it should be equally possibly for this to be inside or outside.

@vanjabucic
Copy link
Author

@fishcakez Sounds like a good idea. Having a pluggable transport would be good as I was also considering using RabbitMQ as a broker.
Anyhow, I will continue using my current branch until the new architecture is in place.
I am interested in creating the new HTTP transport via xhttp and will be following #441 closely.
Thanks

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

Successfully merging this pull request may close these issues.

3 participants