-
Notifications
You must be signed in to change notification settings - Fork 76
Open
Description
StartContainer()
has an ambiguous definition of error signaling:
- the description (presumably) talks about returning gRPC errors (which, in my opinion, is the correct approach):
gnoi/containerz/containerz.proto
Lines 91 to 97 in d04468f
// This RPC will return the following error codes in case of error: // - FailedPrecondition if an invalid value is used for restart policy or // RunAs is supplied. // - AlreadyExists if the container instance name already exists. // - Unavailable if the container tries to expose an already used port. // - Internal if internal container operations are failing. rpc StartContainer(StartContainerRequest) returns (StartContainerResponse) {} - but then
StartContainerResponse
implements its own error signaling mechanism
gnoi/containerz/containerz.proto
Lines 507 to 535 in d04468f
message StartContainerResponse { oneof response { StartOK start_ok = 1; StartError start_error = 2; } } message StartOK { // The running containers name. string instance_name = 1; } message StartError { enum Code { UNSPECIFIED = 0; // An unknown error. The details field should provide more information. UNKNOWN = 1; // The container image was not found. NOT_FOUND = 2; // Exposed port is already used by another container. PORT_USED = 3; } Code error_code = 1; string details = 2; }
This creates ambiguity which mechanism should be used. I'd suggest deprecating the latter.
Similar inband error signaling is defined in UpdateContainer
RPC, although its description does not mention grpc error codes
gnoi/containerz/containerz.proto
Lines 583 to 589 in d04468f
message UpdateContainerResponse { | |
oneof response { | |
UpdateOK update_ok = 1; | |
UpdateError update_error = 2; | |
} | |
} | |
IMO gRPC error codes should be used throughout the service definition and all inband error signaling should be deprecated/removed
Metadata
Metadata
Assignees
Labels
No labels