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

Incompatible with go-kit c5e750d; move to stdlib context #148

Closed
hasLeland opened this issue Feb 4, 2017 · 7 comments
Closed

Incompatible with go-kit c5e750d; move to stdlib context #148

hasLeland opened this issue Feb 4, 2017 · 7 comments

Comments

@hasLeland
Copy link
Contributor

So, with this update to go-kit, truss-generated services no longer compile with tip-of-master go-kit.

The change is to use the version of the context package that's been moved into the go 1.7 standard library. So we need to move (certain) portions of truss generated code to use the context package in the standard library.

@hasAdamr
Copy link
Contributor

hasAdamr commented Feb 4, 2017

Our solution is to vendor go-kit for now. Until go 1.8 comes out and protoc-gen-go updates golang.org/x/net/context to std context. Then everything should be fine.

golang/protobuf#275

@adamryman
Copy link
Member

adamryman commented Mar 31, 2017

The merge has happened!

Need to scope out the work involved in updating based on this. Need to see what go-kit/kit is up to based on this merge.

golang/protobuf#275 (comment)

@adamryman
Copy link
Member

It seems there is yet another dependency that is blocking switching to stdlib "context".

See here grpc/grpc-go#711

@matthewhartstonge
Copy link
Contributor

matthewhartstonge commented Jul 27, 2017

I've been thinking about this issue over the past 2 days.. Personally we are quite keen to get truss to support v0.4.0/v0.5.0 of go-kit.

Currently, from my limited point of view, I think the best way forward may be to shell out different templates based on the version of go-kit you want - perhaps code generated via a go-kit version switch. Keen for the thoughts you have had here...

From our own services and the examples I've played with, it appears that only the grpc transport and server files require golang.org/x/net/context

@adamryman
Copy link
Member

@matthewhartstonge having different templates seems like quite a bit of work to get into the codebase. If you want to take the time to prototype it, we would look it over.

Basically we are using truss in production for our services, and upgrading go-kit currently has no benefit to us, so we don't have the resources to make these changes ourselves.

We also have hope that the switch to standard lib context will happen in golang/protobuf, grpc/grpc-go, and go-kit shortly after type aliases come out in 1.9.

@matthewhartstonge
Copy link
Contributor

@adamryman re: above, PR #216 seeks to implement this.

@adamryman
Copy link
Member

I'm closing this issue as we have a solution in place. See this line https://github.com/TuneLab/truss/pull/224/files#diff-a6509d906716d813562f7d80e091cb00R263

We will probably remove this call in Go 1.10 or Go 1.11, after Go 1.9+ and type alias have become ubiquitous.

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

4 participants