Skip to content

Conversation

@dcantah
Copy link
Member

@dcantah dcantah commented Nov 14, 2025

We should print a clear error message to stderr on if a container can't be found, as well as exiting with a 1 exit code.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

We should print a clear error message to stderr on if a container
can't be found, as well as exiting with a 1 exit code.
@dcantah dcantah force-pushed the change-stop-and-kill-behavior branch from cd90009 to ace9702 Compare November 14, 2025 01:26
@dcantah dcantah marked this pull request as ready for review November 14, 2025 04:43
for id in containerIds {
if let container = containerMap[id] {
containers.append(container)
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need an error for trying to signal a non-running container here?

@Argument(help: "Container IDs")
var containerIds: [String] = []

struct KillError: Error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if this could be instead be moved into ContainerizationClient as public struct PartialSuccessError: Error?

let allContainers = try await ClientContainer.list()
let containerMap = Dictionary(uniqueKeysWithValues: allContainers.map { ($0.id, $0) })

for id in containerIds {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run state checks aren't wanted/needed here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The daemon returns an error if it's not running

}

for (_, err) in error.failed {
allErrors.append("Error from APIServer: \(err)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API server

}

for (_, err) in error.failed {
allErrors.append("Error from APIServer: \(err.localizedDescription)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API server

import ContainerizationOS
import Foundation

struct StandardError: TextOutputStream, Sendable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With #680 you should be able to just use the logger. I think you mentioned a while ago that the StderrLogHandler type I introduced for that isn't needed as there's an existing swift-logging type for logging to stderr, this might be a good time to make that switch.

See also #642.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants