Improvements to the process and network management commands #381
Improvements to the process and network management commands #381dkovba wants to merge 15 commits intoapple:mainfrom
Conversation
9e93b6c to
146e6c5
Compare
# Conflicts: # vminitd/Sources/vminitd/Server+GRPC.swift
7e66d78 to
8633fe6
Compare
| switch err.code { | ||
| case .invalidArgument: | ||
| throw GRPCStatus(code: .invalidArgument, message: "createProcess: failed to create process: \(err)") | ||
| default: |
There was a problem hiding this comment.
Why are we getting lower fidelity on these error codes? Since we are catching a ContainerizationError it's going to have the original code already as that actually happened. Throwing a new error causes us to lose a bit of information. Is there a reason to do this through this PR?
There was a problem hiding this comment.
We are catching ContainerizationError to convert it to GRPCStatus. Without this conversion, all errors would become generic GRPC internal errors.
There was a problem hiding this comment.
It seems reasonable to implement toGRPCStatus in ContainerizationError to reduce the amount of code for error conversion and to convert all possible errors.
There was a problem hiding this comment.
Oh I see now. Many of the codes should map well from cz error to grpc status. You could do something like:
extension ContainerizationError {
/// Convert the ContainerizationError into a GRPCStatus result.
var status: GRPCStatus {
let code: GRPCStatus.Code = {
switch self.code {
case .notFound: .notFound
case .exists: .alreadyExists
case .unsupported: .unimplemented
case .unknown: .unknown
case .internalError: .internalError
case .interrupted: .unavailable
case .invalidState: .failedPrecondition
case .invalidArgument: .invalidArgument
case .timeout: .deadlineExceeded
default: .internalError
}
}()
return GRPCStatus(
code: code,
message: self.description,
cause: self
)
}
}There was a problem hiding this comment.
This approach loses the operation context compared to what I've already implemented. However, we can handle more error codes.
32d6efb to
22d508b
Compare
Improves the process and network management commands.