Skip to content

Feature/support for t compact protocol #1157

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

Merged

Conversation

robertlyson
Copy link
Contributor

Hello,

I took implementation of the TCompactProtocol from current apache thrift.
Also I did some refactoring to this class.
ThriftConnection by default uses a TBinaryProtocol.

I'm just wondering. Isn't it better to take Thrift implementation from nuget?

Let me know in case of any issues.
Thanks!

@Mpdreamz
Copy link
Member

You make a very good point @robertlyson

Would love to move all the thrift types out and take a dependency on http://www.nuget.org/packages/Thrift/, our thrift connection simply predates any of the nuget packages that our out there which is the only reason for not taking a dep :)

@Mpdreamz
Copy link
Member

Mergin this in as is thanks @robertlyson, we might move to one of the thrift nuget packages as you suggested before the final release though 👍

Mpdreamz added a commit that referenced this pull request Dec 22, 2014
…rotocol

Feature/support for t compact protocol
@Mpdreamz Mpdreamz merged commit 54ad780 into elastic:develop Dec 22, 2014
@robertlyson robertlyson deleted the feature/SupportForTCompactProtocol branch January 27, 2015 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants