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

Add grpc.ForceCodec-like ServerOption based on encoding.Codec #3694

Closed
nadilas opened this issue Jun 15, 2020 · 8 comments · Fixed by #3698 or #4205
Closed

Add grpc.ForceCodec-like ServerOption based on encoding.Codec #3694

nadilas opened this issue Jun 15, 2020 · 8 comments · Fixed by #3698 or #4205

Comments

@nadilas
Copy link

nadilas commented Jun 15, 2020

What version of gRPC are you using?

google.golang.org/grpc v1.29.1

What version of Go are you using (go version)?

go1.14.3 darwin/amd64

What did you do?

Following up on #2189, the server side was missed, it is still using the deprecated grpc.Codec interface.

What did you expect to see?

// CustomCodec returns a ServerOption that sets a codec for message marshaling and unmarshaling.
//
// This will override any lookups by content-subtype for Codecs registered with RegisterCodec.
func CustomCodec(codec encoding.Codec) ServerOption {
	return newFuncServerOption(func(o *serverOptions) {
		o.codec = codec
	})
}

What did you see instead?

// CustomCodec returns a ServerOption that sets a codec for message marshaling and unmarshaling.
//
// This will override any lookups by content-subtype for Codecs registered with RegisterCodec.
func CustomCodec(codec Codec) ServerOption {
	return newFuncServerOption(func(o *serverOptions) {
		o.codec = codec
	})
}



// Codec defines the interface gRPC uses to encode and decode messages.
// Note that implementations of this interface must be thread safe;
// a Codec's methods can be called from concurrent goroutines.
//
// Deprecated: use encoding.Codec instead.
type Codec interface {
	// Marshal returns the wire format of v.
	Marshal(v interface{}) ([]byte, error)
	// Unmarshal parses the wire format into v.
	Unmarshal(data []byte, v interface{}) error
	// String returns the name of the Codec implementation.  This is unused by
	// gRPC.
	String() string
}
@dfawley
Copy link
Member

dfawley commented Jun 18, 2020

I believe CustomCodec should be marked deprecated, and there is no need to provide an alternative using encoding.Codec directly. Let me know if you have a use case for it.

@nadilas
Copy link
Author

nadilas commented Jun 19, 2020

Same use case as mentioned in the linked issue: #2189

I am trying to use https://github.com/mwitkow/grpc-proxy

func ExampleRegisterService() {
	// A gRPC server with the proxying codec enabled.
	server := grpc.NewServer(grpc.CustomCodec(proxy.Codec()))
	// Register a TestService with 4 of its methods explicitly.
	proxy.RegisterService(server, director,
		"mwitkow.testproto.TestService",
		"PingEmpty", "Ping", "PingError", "PingList")
}

@dfawley
Copy link
Member

dfawley commented Jun 26, 2020

I think we should keep this open to track adding a non-deprecated version of the API. The server-side use case seems just as appropriate as the client-side.

@dfawley dfawley reopened this Jun 26, 2020
@dfawley dfawley changed the title grpc.CustomCodec uses deprecated interface Add grpc.ForceCodec-like ServerOption based on encoding.Codec Jun 26, 2020
@ash2k
Copy link
Contributor

ash2k commented Dec 27, 2020

Same issue here - I want to use a custom codec for a particular gRPC server. I have more than one server in the program and ideally I'd prefer not to register a global codec. Also, I wonder what is the reasoning for adding global registries for codec and compressor? Allowing a per-server configuration is much more flexible and there is no thread-safety issues (global registries are not thread safe at the moment).

@ash2k
Copy link
Contributor

ash2k commented Dec 27, 2020

Sorry, more context: servers that I have should use different codecs for the same content type - global registry makes this hard to achieve. I can work this around, but it's not as clean as it could be with this issue resolved.

@ash2k
Copy link
Contributor

ash2k commented Apr 26, 2021

I've opened a PR to add an option. Would be great to get some eyes on it. Thanks.

@FZambia
Copy link

FZambia commented May 1, 2021

I have a similar use case as @ash2k (several GRPC servers inside one program which must use different codecs for the same content type), really unfortunate to not have a non-deprecated way to set it on per-server basis.

@easwars
Copy link
Contributor

easwars commented May 4, 2021

Moving the ownership of the issue to @dfawley who is reviewing #4205.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants