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

Gogo support? #19

Closed
maros7 opened this issue Sep 7, 2018 · 6 comments
Closed

Gogo support? #19

maros7 opened this issue Sep 7, 2018 · 6 comments

Comments

@maros7
Copy link

maros7 commented Sep 7, 2018

Hi, would you consider adding official support for https://github.com/gogo/protobuf? Using inprocgrpc causes panic when .protos are compiled with gogo/protobuf and not golang/protobuf. I can of course create my own version of inprocgrpc, but would be good I guess to have it in this repository. I can look into submitting a PR.

@jhump
Copy link
Contributor

jhump commented Sep 7, 2018

I do not want to add any dependency from packages in this repo to gogo/protobuf -- that is just dependency baggage/bloat for people that use golang/protobuf.

One option would be to make the message cloning pluggable. That way your code could provide a custom cloner that used gogo/protobuf without adding it as a dependency to inprocgrpc itself. I'll look into that.

Also note that this will likely be addressed in the future: one of the objectives of the v2 API of the protobuf runtime is to support other kinds of proto messages -- such as those generated by gogo's protoc plugin, or even dynamic messages.

@maros7
Copy link
Author

maros7 commented Sep 7, 2018

Fair enough. And thanks for the detailed answer. I’ll just create a custom channel for now.

@jhump
Copy link
Contributor

jhump commented Sep 7, 2018

@maros7, can you try pull down my branch for #22 and verify whether that works for you?

I think you'll need a custom cloner that looks basically likes like so:

import (
	"fmt"
	"reflect"

	"github.com/gogo/protobuf/proto"
)

type gogoCloner struct{}

func (gogoCloner) Copy(out, in interface{}) error {
	if reflect.TypeOf(in) != reflect.TypeOf(out) {
		return fmt.Errorf("type mismatch: %T != %T", in, out)
	}
	if reflect.ValueOf(out).IsNil() {
		return fmt.Errorf("out must not be nil")
	}
	inM := in.(proto.Message)
	outM := out.(proto.Message)
	outM.Reset()
	proto.Merge(outM, inM)
	return nil
}

func (gogoCloner) Clone(in interface{}) (interface{}, error) {
	return proto.Clone(in.(proto.Message)), nil
}

@maros7
Copy link
Author

maros7 commented Sep 7, 2018

Thanks. I will try this and get back to you.

@maros7
Copy link
Author

maros7 commented Sep 10, 2018

Hey. Yes, this solves my issue. Thanks!

@jhump jhump closed this as completed in #22 Sep 10, 2018
@jhump
Copy link
Contributor

jhump commented Sep 10, 2018

@maros7, I just merged that branch. But be aware that I made some non-trivial changes to the Cloner interface. I revised the example above so that it should work -- just make sure you grab the latest changes to that gogo example. (Most urgently, I swapped the order of args to the Copy method, so its signature is now consistent with the copy builtin.)

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

2 participants