Skip to content

client: refactor ContainerWait to use client defined options/results structs#51312

Merged
thaJeztah merged 1 commit intomoby:masterfrom
austinvazquez:refactor-client-container-wait
Oct 29, 2025
Merged

client: refactor ContainerWait to use client defined options/results structs#51312
thaJeztah merged 1 commit intomoby:masterfrom
austinvazquez:refactor-client-container-wait

Conversation

@austinvazquez
Copy link
Contributor

- What I did
#50965

This change refactors the client ContainerWait implementation to take and return client defined options/result structs. This is to decouple the client from API changes.

- How I did it

- How to verify it

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

…structs

Signed-off-by: Austin Vazquez <austin.vazquez@docker.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the ContainerWait API to use structured options and results instead of passing a raw condition string and returning separate channels. The refactoring introduces ContainerWaitOptions to encapsulate the wait condition and ContainerWaitResult to bundle the result and error channels together.

Key Changes:

  • Introduced ContainerWaitOptions struct with a Condition field to replace the raw container.WaitCondition parameter
  • Introduced ContainerWaitResult struct containing Results and Errors channels to replace the separate channel return values
  • Updated the ContainerWait method signature across client implementation and interface definition

Reviewed Changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
client/container_wait.go Defines new ContainerWaitOptions and ContainerWaitResult types; updates ContainerWait implementation to use these structs
client/client_interfaces.go Updates ContainerAPIClient interface signature for ContainerWait
client/container_wait_test.go Updates unit tests to use new API with ContainerWaitOptions{} and access channels via wait.Results and wait.Errors
integration/container/wait_test.go Updates integration tests to use the refactored API
integration/container/daemon_linux_test.go Updates daemon integration test to use the refactored API
integration/container/create_test.go Updates container creation test to use the refactored API
integration-cli/docker_api_containers_test.go Updates CLI integration test to use the refactored API

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +21 to +25
// ContainerWaitResult defines the result of a container wait operation.
type ContainerWaitResult struct {
Results <-chan container.WaitResponse
Errors <-chan error
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this always returns a single result (it blocks until it's reaching the desired state).

But I may have a look at follow-ups; wondering if we should

  • add an explicit Timeout option in the options; this would set context.WithTimeout under the hood.
  • abstract away the channel, and just make the function blocking
  • but we may need to have some options for an Ack() method for situations where the wait must be registered before starting the container 🤔

@thaJeztah thaJeztah merged commit d3eb04f into moby:master Oct 29, 2025
234 of 235 checks passed
@austinvazquez austinvazquez deleted the refactor-client-container-wait branch October 29, 2025 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/go-sdk kind/refactor PR's that refactor, or clean-up code module/client release-blocker PRs we want to block a release on status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants