Skip to content

add StopCtx function, to stop containers with context#1094

Open
GRbit wants to merge 1 commit intoorlangure:masterfrom
GRbit:master
Open

add StopCtx function, to stop containers with context#1094
GRbit wants to merge 1 commit intoorlangure:masterfrom
GRbit:master

Conversation

@GRbit
Copy link

@GRbit GRbit commented Nov 22, 2024

I needed that code in my use case, hope it could be useful to other users.

Copy link
Owner

@orlangure orlangure left a comment

Choose a reason for hiding this comment

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

Great idea, thank you for working on this.

Did you make sure the context is propagated all the way to the docker client that actually cancels things?


defer func() { _ = g.log.Sync() }()

eg, eCtx := errgroup.WithContext(ctx)
Copy link
Owner

Choose a reason for hiding this comment

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

I think that we don't need two copies of the same function. Instead, we need the Stop function to call StopCtx with the background context, and StopCtx should be the only function that has all the errgroup code.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I've checked that github.com/docker/docker/client.ContainerStop and github.com/docker/docker/client.ContainerRemove both take the same context. Though, I didn't this behavior specifically.

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