tart exec: explain that Tart Guest Agent is required #1078
Conversation
Also handle decrease the connection timeout to 1 second and provide a hint to the user.
There was a problem hiding this comment.
Pull Request Overview
This PR clarifies the requirement for the Tart Guest Agent, tightens the gRPC connection timeout to 1 second, and surfaces a hint when the agent isn’t running.
- Adds a
discussionsection to theExeccommand configuration about the Tart Guest Agent. - Refactors execution into a separate
executemethod with a 1-secondCallOptionstimeout. - Catches
GRPCConnectionPoolErrorand throws a more user-friendly hint.
Comments suppressed due to low confidence (1)
Sources/tart/Commands/Exec.swift:82
- Add tests to cover the new timeout behavior, such as verifying that a timeout error is raised when the VM agent is unresponsive.
let callOptions = CallOptions(timeLimit: .timeout(.seconds(1)))
| Requires Tart Guest Agent running in a guest VM. | ||
| Note that all non-vanilla Cirrus Labs VM images already have the Tart Guest Agent installed. |
There was a problem hiding this comment.
[nitpick] Rephrase the discussion for clarity and consistency, e.g., "Requires the Tart Guest Agent to be running inside the VM." Also remove the leading blank line to avoid unintended formatting.
| Requires Tart Guest Agent running in a guest VM. | |
| Note that all non-vanilla Cirrus Labs VM images already have the Tart Guest Agent installed. | |
| Requires the Tart Guest Agent to be running inside the VM. | |
| Note that all non-vanilla Cirrus Labs VM images already have the Tart Guest Agent installed. |
Sources/tart/Commands/Exec.swift
Outdated
| func execute(_ channel: GRPCChannel) async throws { | ||
| let agentAsyncClient = AgentAsyncClient(channel: channel) | ||
| let execCall = agentAsyncClient.makeExecCall() | ||
| let callOptions = CallOptions(timeLimit: .timeout(.seconds(1))) |
There was a problem hiding this comment.
Extract the hardcoded timeout (1 second) into a named constant or configuration parameter to avoid magic numbers and simplify future adjustments.
There was a problem hiding this comment.
Looks fine to me.
Sources/tart/Commands/Exec.swift
Outdated
| do { | ||
| try await execute(channel) | ||
| } catch let error as GRPCConnectionPoolError { | ||
| throw RuntimeError.Generic("Failed to connect to the VM using its control socket, is Tart Guest Agent running?") |
There was a problem hiding this comment.
Include the original error details (e.g., error.localizedDescription) in this message to help users diagnose the underlying cause.
| throw RuntimeError.Generic("Failed to connect to the VM using its control socket, is Tart Guest Agent running?") | |
| throw RuntimeError.Generic("Failed to connect to the VM using its control socket, is Tart Guest Agent running? Error details: \(error.localizedDescription)") |
Also handle decrease the connection timeout to 1 second and provide a hint to the user.