-
Notifications
You must be signed in to change notification settings - Fork 4k
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
THRIFT-4215/4216 Make transport factories return errors #1285
Conversation
This commit changes the signature of TTransportFactory.GetTransport from GetTransport(trans TTransport) TTransport to GetTransport(trans TTransport) (TTransport, error) so the factory can pass any underlying error to the caller (previously such errors were ignored). This is a backwards incompatible change for anyone implementing custom transports, but it shouldn't effect anyone using the ones in this library. Fixes THRIFT-4215. Fixes THRIFT-4216.
Closing PR until all tests are fixed, I don't want to waste Apache's Travis capacity. |
Client: Go Patch: D. Can Celasun <can@dcc.im> This closes #1285
For the record, this caused build failures in Go kit; I don't imagine my usage is particularly strange, but maybe it is. (Link.) |
Yes sorry about that, this was an expected breakage as the interface has changed. I suggest using EDIT: Just realized this is for go-kit, so you can't really vendor thrift. Maybe a note in the README might be useful. |
Client: Go Patch: D. Can Celasun <can@dcc.im> This closes apache#1285
Client: Go Patch: D. Can Celasun <can@dcc.im> This closes apache#1285
Client: Go Patch: D. Can Celasun <can@dcc.im> This closes apache#1285
Client: Go Patch: D. Can Celasun <can@dcc.im> This closes apache#1285
Client: Go Patch: D. Can Celasun <can@dcc.im> This closes apache#1285
This commit changes the signature of
TTransportFactory.GetTransport
fromto
so the factory can pass any underlying error to the caller (previously such errors were ignored).
This is a backwards incompatible change for anyone implementing custom transports, but it shouldn't effect anyone using the ones in this library.
Fixes THRIFT-4215.
Fixes THRIFT-4216.
P.S: Some files have formatting-only changes due to
gofmt
.