I've been working on a Node based plugin and was testing to see if my health service is working, and I noticed that the current GRPCClient implementation for Ping() doesn't actually evaluate if the service is healthy (SERVING):
|
func (c *GRPCClient) Ping() error { |
|
client := grpc_health_v1.NewHealthClient(c.Conn) |
|
_, err := client.Check(context.Background(), &grpc_health_v1.HealthCheckRequest{ |
|
Service: GRPCServiceName, |
|
}) |
|
|
|
return err |
|
} |
Instead of just checking for err (which would tell you if the service was "hard down", or didn't implement /grpc.health.v1.Health/Check), this should also check the response from client.Check to check that it returned grpc_health_v1.HealthCheckResponse_SERVING.
Changing this would probably break a lot of people's stuff right now, but I thought I'd point it out.
I've been working on a Node based plugin and was testing to see if my health service is working, and I noticed that the current GRPCClient implementation for
Ping()doesn't actually evaluate if the service is healthy (SERVING):go-plugin/grpc_client.go
Lines 127 to 134 in cfdf485
Instead of just checking for
err(which would tell you if the service was "hard down", or didn't implement/grpc.health.v1.Health/Check), this should also check the response fromclient.Checkto check that it returnedgrpc_health_v1.HealthCheckResponse_SERVING.Changing this would probably break a lot of people's stuff right now, but I thought I'd point it out.