Fix stop kill missing containers #979
Conversation
|
@dcantah |
| let allContainers = try await ClientContainer.list() | ||
| let containers = try self.all | ||
| ? allContainers | ||
| : Self.containers(matching: containerIds, in: allContainers) |
There was a problem hiding this comment.
I think we should handle ambiguous prefix matches here.
Currently, allContainers.first(where:) will silently pick the first matching container. If I have containers abc and abd, running stop ab might stop abc just because it appears first in the list.
It would be safer to filter all matches and throw an error if matches.count > 1 (unless there's an exact match). This aligns with how Docker handles ambiguous prefixes.
|
|
||
| @testable import ContainerCommands | ||
|
|
||
| struct ContainerStopValidationTests { |
There was a problem hiding this comment.
Could we add a test case for ambiguous prefixes?
We should verify that stop ab throws an error if both abc and abd exist. It's an important edge case to prevent accidental stops.
| import ContainerizationOS | ||
| import Foundation | ||
|
|
||
| package protocol ContainerIdentifiable { |
There was a problem hiding this comment.
Refactoring this resolution logic into a shared helper is a great move. It keeps Stop and Kill consistent and makes testing way easier.
2 participants
Type of Change
Motivation and Context
The
stopandkillcommands currently do not gracefully handle cases where container IDs are missing or not found. This PR adds proper error handling for missing containers and ensures validation tests run correctly.Fixes #878
Testing