[
https://issues.apache.org/jira/browse/THRIFT-4219?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16044556#comment-16044556
]
Jens Geyer edited comment on THRIFT-4219 at 6/9/17 3:41 PM:
------------------------------------------------------------
{quote}
I've checked the Java and Python clients, neither of them support a GET-based
implementation. So AFAICT, my suggested changes would make it more, not less,
consistent with other languages. Thoughts?
{quote}
Sure. The first thought is that I sometimes answer/comment two questions/topics
in one post. :-)
- The consistency was related to syntax and semantics.
- The HTTP POST vs. GET was my personal opinion that I am fine with throwing
out GET because:
-- Go is the only part of the library that implements it (for reasons that
I'm not aware of) and
-- potentially large requests HTTP should be made by POST anyway and
-- supporting GET and POST may make it more prone to errors
----
{quote}
I very well may be missing something, but I do not see how this behavior is
consistent across the library. Is the behavior throughout the library that a
call to Open can return a nil error, when the transport was not opened? Are
there any other one-shot transports, and if so, is there a clear set of rules
around which factory functions return one-shot transports, and which ones do
not?
{quote}
I was talking about https://thrift.apache.org/static/files/thrift-20070401.pdf
in particular section 3.1 and 4.1. I was not talking about implemntation
details like returning an {{result, error}} tuple or the like. Because that is
considered idiomatic and state of the art in Golang, it is very likely the best
way to implement it. Whether or not a function should return {{nil}} in a
certain case or whether that makes sense or not is a different question.
The careful observer may have noticed that I did not vote in any direction, I
just wanted to remind everybody about the overall design principles Thrift is
built upon. Not more, not less.
was (Author: jensg):
{quote}
I've checked the Java and Python clients, neither of them support a GET-based
implementation. So AFAICT, my suggested changes would make it more, not less,
consistent with other languages. Thoughts?
{quote}
Sure. The first thought is that I sometimes answer/comment two questions/topics
in one post.
- The consistency was related to syntax and semantics.
- The HTTP POST vs. GET was my personal opinion that I am fine with throwing
out GET because
a) Go is the only part of the library that implements it (for reasons that
I'm not aware of) and
b) because potentially large requests HTTP should be made by POST anyway and
c) because supporting GET and POST may make it more prone to errors
{quote}
I very well may be missing something, but I do not see how this behavior is
consistent across the library. Is the behavior throughout the library that a
call to Open can return a nil error, when the transport was not opened? Are
there any other one-shot transports, and if so, is there a clear set of rules
around which factory functions return one-shot transports, and which ones do
not?
{quote}
I was talking about https://thrift.apache.org/static/files/thrift-20070401.pdf
in particular section 3.1 and 4.1. I was not talking about implemntation
details like returning an {{result, error}} tuple or the like. Because that is
considered idiomatic and state of the art in Golang, it is very likely the best
way to implement it. Whether or not a function should return {{nil}} in a
certain case or whether that makes sense or not is a different question.
The careful observer may have noticed that I did not vote in any direction, I
just wanted to remind everybody about the overall design principles Thrift is
built upon. Not more, not less.
> Golang Http Clients Created with Nil Buffer
> -------------------------------------------
>
> Key: THRIFT-4219
> URL: https://issues.apache.org/jira/browse/THRIFT-4219
> Project: Thrift
> Issue Type: Bug
> Components: Go - Library
> Reporter: James Mouradian
>
> As of 2b1b32c,
> [{{NewTHttpClientWithOptions}}|https://github.com/apache/thrift/blob/master/lib/go/thrift/http_client.go#L93]
> creates a {{THttpClient}} instance with a nil {{requestBuffer}}. This causes
> a panic when the instance attempts to write to the buffer.
> A patch probably fits well with THRIFT-4215 and THRIFT-4216.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
