Socket for run and start commands, and basic socket protocol#155
Conversation
marciniwanicki
left a comment
There was a problem hiding this comment.
Big, big thanks @beefon! The changes look great. Left few small comments but overall it works well.
| vmSocketServer: r.resolve(VMSocketServer.self) | ||
| ) | ||
| } | ||
| registry.register(VMSocketServer.self) { _ in |
There was a problem hiding this comment.
Maybe SocketServer and DefaultSocketServer to be aligned with the naming conventions of other utilities.
| import Foundation | ||
|
|
||
| final class MakeScreenshotRequestProcessor { | ||
| private let vmScreenshotter: VMScreenshotter |
There was a problem hiding this comment.
screenshotter var will be more aligned with other "VM" variable types, e.g. bundle not vmBundle.
| } | ||
|
|
||
| func process(request: MakeScreenshotPayload) -> PromisedSocketResponse { | ||
| DispatchQueue.main.sync { |
There was a problem hiding this comment.
Maybe in the future we can try to use async await.
| public func terminateVmAndCurrentProcess( | ||
| machineStateURL: URL | ||
| machineStateURL: URL, | ||
| postFlight: @escaping (Result<Void, Error>) -> Void = { _ in } |
There was a problem hiding this comment.
What do we do with this result? Looks unused at the moment.
There was a problem hiding this comment.
Currently it is used to provide feedback over socket whether VM has been terminated or failed, this is inside TerminateVmRequestProcessor:
vm.terminateVmAndCurrentProcess(machineStateURL: vmBundle.machineState.asURL) { terminationResult in
switch terminationResult {
case .success:
settableResponse.set(response: .success([:]))
case let .failure(error):
settableResponse.set(response: .error("Failed to terminate VM: \(error)"))
}
socketQueue.sync(flags: .barrier) {}
}
There was a problem hiding this comment.
Ah missed that, thank you.
|
Thank you @beefon for all the updates, you should be able to merge it now. |

In this PR
startandruncommands are getting optional--start-socket-atCLI arg. If it is set, then socket is created at specified path.New
socketcommand allows you to send a JSON contents into socket. JSON has to conformCurieSocketRequest. Help for the command has some examples:I've also added 3 payloads, one is for testing (ping; i'm happy to remove it in case you don't like to have it around) and two with actual feature (terminating the VM and making screenshots).
Happy to split screenshot making feature to a separate PR if you want, but i found it rather small.
Example usage:
Terminating VM
Making screenshot
I've tried to make
UnixDomainSocketProtocol's JSON as human friendly as possible with extensibility for the future needs.