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

Package is broken against github.com/apache/thrift/lib/go/thrift #68

Closed
peterbourgon opened this issue Jul 22, 2017 · 15 comments
Closed

Comments

@peterbourgon
Copy link

peterbourgon commented Jul 22, 2017

blep ~/src/github.com/go-kit/kit (master) go get -u -v github.com/openzipkin/zipkin-go-opentracing/...
 ...
 ...
 ...
github.com/openzipkin/zipkin-go-opentracing/thrift/gen-go/scribe
# github.com/openzipkin/zipkin-go-opentracing/thrift/gen-go/scribe
../../openzipkin/zipkin-go-opentracing/thrift/gen-go/scribe/scribe.go:147:28: cannot use scribeProcessorLog literal (type *scribeProcessorLog) as type thrift.TProcessorFunction in assignment:
        *scribeProcessorLog does not implement thrift.TProcessorFunction (wrong type for Process method)
                have Process(int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
                want Process(context.Context, int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
../../openzipkin/zipkin-go-opentracing/thrift/gen-go/scribe/scribe.go:157:27: not enough arguments in call to processor.Process
        have (int32, thrift.TProtocol, thrift.TProtocol)
        want (context.Context, int32, thrift.TProtocol, thrift.TProtocol)
@basvanbeek
Copy link
Member

basvanbeek commented Jul 23, 2017

ugh so they just added context to the thrift Go library but to compile a compatible thrift file the latest compiler release does not include the proper switch to enable context for generated Go code. Trying to install bleeding edge compiler in the hope it fixes the issue.

@basvanbeek
Copy link
Member

Super... even using latest thrift Compiler it fails:

https://issues.apache.org/jira/browse/THRIFT-4260

@basvanbeek
Copy link
Member

It compiles but is wrong. *ScribeClient should implement Scribe interface

@basvanbeek basvanbeek reopened this Jul 23, 2017
@peterbourgon
Copy link
Author

Is it worth reaching out to them and asking why the went with the deprecated package?

@basvanbeek
Copy link
Member

They didn't it was my mistake. My editor opened the wrong file. They have x/context for Go versions not supporting the stdlib one.

@andriylesch
Copy link

andriylesch commented Jul 24, 2017

Hello Guys,
I have the same problem when i am trying to run my service i have error

mydirectory/github.com/openzipkin/zipkin-go-opentracing/_thrift/gen-go/scribe
vendor/github.com/openzipkin/zipkin-go-opentracing/_thrift/gen-go/scribe/scribe.go:147: cannot use scribeProcessorLog literal (type *scribeProcessorLog) as type thrift.TProcessorFunction in assignment:
	*scribeProcessorLog does not implement thrift.TProcessorFunction (wrong type for Process method)
		have Process(int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
		want Process(context.Context, int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
vendor/github.com/openzipkin/zipkin-go-opentracing/_thrift/gen-go/scribe/scribe.go:157: not enough arguments in call to processor.Process
	have (int32, thrift.TProtocol, thrift.TProtocol)
	want (context.Context, int32, thrift.TProtocol, thrift.TProtocol)

when you are planing to fix it?
Thank you in advance for your answer.

@basvanbeek
Copy link
Member

currently Thrift for Go is broken and is being worked on... once this is done I can fix on our end...

@dcelasun
Copy link

dcelasun commented Jul 25, 2017

Hi everyone, a fix was just merged upstream, could you try with the latest compiler from master?

For future reference, using master for either the library or the compiler assumes you are using it for the other one as well. Officially, context support will be included in the upcoming 0.11 release.

@basvanbeek
Copy link
Member

updated with PR #70...

@jayeshkhairnar
Copy link

jayeshkhairnar commented Sep 11, 2017

*metaProcessorHealth does not implement thrift.TProcessorFunction (wrong type for Process method)
                have Process(int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
                want Process(context.Context, int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
src/github.com/uber/tchannel-go/thrift/gen-go/meta/meta.go:290: cannot use metaProcessorThriftIDL literal (type *metaProcessorThriftIDL) as type thrift.TProcessorFunction in assignment:
        *metaProcessorThriftIDL does not implement thrift.TProcessorFunction (wrong type for Process method)
                have Process(int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
                want Process(context.Context, int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
src/github.com/uber/tchannel-go/thrift/gen-go/meta/meta.go:291: cannot use metaProcessorVersionInfo literal (type *metaProcessorVersionInfo) as type thrift.TProcessorFunction in assignment:
        *metaProcessorVersionInfo does not implement thrift.TProcessorFunction (wrong type for Process method)
                have Process(int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
                want Process(context.Context, int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
src/github.com/uber/tchannel-go/thrift/gen-go/meta/meta.go:301: not enough arguments in call to processor.Process
        have (int32, thrift.TProtocol, thrift.TProtocol)
        want (context.Context, int32, thrift.TProtocol, thrift.TProtocol)

Above error I am getting

@basvanbeek
Copy link
Member

@jayeshkhairnar please file your issue with github.com/uber/tchannel-go as the code they're using is not compatible anymore with master on github.com/apache/thrift/lib/go/thrift

In the mean time you can use vendoring to provide tchannel-go with an older version / tagged release of thrift/lib/go/thrift for your application

@tpiecora
Copy link

tpiecora commented Sep 30, 2017

@basvanbeek
Getting this error when trying to build:

	*scribeProcessorLog does not implement thrift.TProcessorFunction (wrong type for Process method)
		have Process(context.Context, int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
		want Process(int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
vendor/github.com/openzipkin/zipkin-go-opentracing/thrift/gen-go/scribe/scribe.go:341:29: too many arguments in call to processor.Process
	have (context.Context, int32, thrift.TProtocol, thrift.TProtocol)
	want (int32, thrift.TProtocol, thrift.TProtocol)

I believe this is referencing the same issue: uber/tchannel-go#648
Sounds like they have no plans for addressing this.

@basvanbeek
Copy link
Member

@tpiecora you have an older version the thrift go library. You can fix by updating:

go get -u github.com/apache/thrift/lib/go/thrift

uber/tchannel-go is compatible with an older version of the thrift go library. Zipkin-go-opentracing tries to stay current with latest commits on master from its dependencies. If you desire to use older / tagged releases of these libraries you have to resort to vendoring. You can always use a tagged release from this repo.

@kingeasternsun
Copy link

cannot use agentProcessorEmitBatch literal (type *agentProcessorEmitBatch) as type thrift.TProcessorFunction in assignment:
*agentProcessorEmitBatch does not implement thrift.TProcessorFunction (wrong type for Process method)
have Process(int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)
want Process(context.Context, int32, thrift.TProtocol, thrift.TProtocol) (bool, thrift.TException)

@basvanbeek
Copy link
Member

can you please update the thrift-go package, looks like you have an older one in your gopath:

go get -u github.com/apache/thrift/lib/go/thrift

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

No branches or pull requests

7 participants