Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Create a context for the progress printer #764

Closed
wants to merge 1 commit into from
Closed

Create a context for the progress printer #764

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion internal/commands/build/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,9 @@ func buildImageUsingBuildx(app *types.App, contextPath string, opt buildOptions,
out = os.NewFile(dockerCli.Out().FD(), "/dev/stdout")
}

pw := progress.NewPrinter(ctx, out, opt.progress)
ctx2, cancel := context.WithCancel(context.TODO())
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a regression test could be tricky to add here, but maybe a one line comment explaining why we have another context?

Copy link
Member

Choose a reason for hiding this comment

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

maybe we can find a better name, like printerContext?

Suggested change
ctx2, cancel := context.WithCancel(context.TODO())
printerContext, cancel := context.WithCancel(context.TODO())

defer cancel()
pw := progress.NewPrinter(ctx2, out, opt.progress)

// We rely on buildx "docker" builder integrated in docker engine, so don't need a DockerAPI here
resp, err := build.Build(ctx, driverInfo, buildopts, nil, dockerCli.ConfigFile(), pw)
Expand Down