Skip to content

Conversation

@dmcgowan
Copy link

@dmcgowan dmcgowan commented Aug 15, 2025

Signed-off-by: Derek McGowan <derek@mcg.dev>
@thaJeztah thaJeztah force-pushed the support-native-docker-grpc branch from 8f5fb52 to 36788c6 Compare December 12, 2025 10:55
@thaJeztah
Copy link
Member

Did a quick rebase to get a fresh run of CI, and moved it out of draft after moby/moby#50744 was merged / accepted.

Comment on lines +83 to +86
if err != nil {
opts = append([]client.ClientOpt{
client.WithContextDialer(func(context.Context, string) (net.Conn, error) {
return d.Dial(ctx)
Copy link
Member

Choose a reason for hiding this comment

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

Should we add some comment to describe that this is the fallback for the legacy approach @akerouanton (Which could include a link to the moby PR), so that we know when we could potentially remove the fallback?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that'd make sense.

Comment on lines +74 to +83
c, err = client.New(ctx, d.DockerAPI.DaemonHost(), opts...)
workers []*client.WorkerInfo
)
if err == nil {
workers, err = c.ListWorkers(ctx)
if err != nil {
c.Close()
}
}
if err != nil {
Copy link

Choose a reason for hiding this comment

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

Drive by review:

Is this not missing a return after c.Close()? Here the second if err != nil { can happen after ListWorkers and it does not look like that is the actual intent.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants