Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
@jeking3 done
---
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
It bugged me to have a time.Sleep in the test so I wrote the retry logic :)
---
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
It's not clear to me what went wrong in the Travis build. Trusty, for
example, seems to have stack overflow'd while installing ocaml. Is there a way
to retrigger a build without pushing another
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
Hmm yeah must be a race condition with the test server starting in the go
routine. Any thoughts on how to wait for it to start up? I guess I could use a
wait group to wait at least until
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
@dcelasun I don't think I can do a `runtime.Version` check since the code
is failing at compilation time. I'll just remove that line.
---
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
@dcelasun: done, could you take another look?
---
GitHub user johnboiles opened a pull request:
https://github.com/apache/thrift/pull/1488
Golang -remote.go client cleanup
Several changes here related to the generated `-remote.go` clients
* Serialize cli input with TJsonProtocol since json->thrift serializat
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
@dcelasun I'm happy to do that. Though in my experience, tests that depend
on timing will find a way to be flakey, especially in CI.
If you think the value of testing this is worth
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
@jeking3 @dcelasun Travis is green so I think we're almost good to go here.
Mind taking another look and merging if it looks good?
---
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
I think that's the right approach. Itâs pretty easy/fast to install
golang from a tar.gz distribution. I do it pretty frequently to pin a specific
version of go in docker images.
I'll
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
Sounds good want me to take a stab at dropping 1.6 support?
---
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
> For Thrift, PRs are not merged, just closed. You'll see a commit with you
as the author but someone else as the committer, like these.
Ah got it, good to know thanks!
---
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
Travis was green so i've squashed it!
FYI for the future: GitHub can now automatically squash commits in PRs at
merge time. If that feature is enabled for the repo it shouldn't
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
Done ^. Thanks again for all your direction Dcelasun
---
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
Good call, so then:
```
func (p *BaseClient) Client_() thrift.TClient {
return p.c
}
```
?
---
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
This is a lot cleaner -- I like it. We still have the same issue with
tests, though, if the base service is in a separate package -- you can't access
the client as `.c` on the base service since
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
You're correct that it's a service extending another service; tbh I missed
that too (I'm not the author of the `.thrift` files. I'm also generally new to
thrift so still sorting out what should
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
> Are you getting this second "Base" struct with 0.11 compiler? Because I
can't reproduce.
I've been working off the master branch, but I just tried generati
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
Thank you for the explainer @dcelasun! Makes sense to me.
@Jens-G, @jeking3: if one of you sign off on this, I'll make the changes to
drop go1.6 and `x/net/context` support from
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
I really appreciate your feedback & involvement @dcelasun! I personally
think dropping `go1.6`/`x/net/context` support is best for the project. Are you
the right person to make that
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
@dcelasun any thoughts on fixing the tests? Looks like
`tutorial/go/src/tutorial/tutorial.go` imports its `BaseClient` from another
package. Since `c` isn't public in that other package, my
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1461
> Why are they deprecated? I probably only overlook sth, so bear with me
That's a great question. Looks like they were marked deprecated by
@dcelasun in #1382 as a part of THRIFT-4
Github user johnboiles commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1382#discussion_r160477076
--- Diff: compiler/cpp/src/thrift/generate/t_go_generator.cc ---
@@ -1953,177 +1960,75 @@ void
t_go_generator::generate_service_client(t_service
GitHub user johnboiles opened a pull request:
https://github.com/apache/thrift/pull/1461
Golang: fix for (deprecated) New*ClientFactory and New*ClientProtocol
functions
Latest thrift:master can cause panics when using deprecated
`New*ClientFactory` and `New*ClientProtocol
Github user johnboiles commented on the issue:
https://github.com/apache/thrift/pull/1459
Looks like two of the builds failed as expected on go1.6 context. The third
build that failed looks like it failed on JS issues, so likely something flakey.
---
GitHub user johnboiles opened a pull request:
https://github.com/apache/thrift/pull/1459
Golang: do something with context.Context
This patch wires through `context.Context` such that it can be used in in
`http.Request`'s `WithContext` method. This allows Thrift HTTP requests
26 matches
Mail list logo
