-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass context to drivers during bundle operation #221
Conversation
When we execute a bundle operation, pass a context so that they can handle cancellations and timeouts. Signed-off-by: Carolyn Van Slyck <carolyn.vanslyck@microsoft.com>
@@ -78,7 +79,7 @@ func (a Action) Run(c claim.Claim, creds credentials.Set, opCfgs ...OperationCon | |||
} | |||
|
|||
var opErr *multierror.Error | |||
opResult, err := a.Driver.Run(op) | |||
opResult, err := a.Driver.Run(context.TODO(), op) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still working on how we want to introduce the context, leaning towards the first parameter to Action.Run
unless there's a reason for opCfgs to be involved somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making context the first parameter is probably most idiomatic. From the package docs:
Do not store Contexts inside a struct type; instead, pass a Context explicitly to each function that needs it. The Context should be the first parameter, typically named ctx
@@ -32,7 +33,7 @@ type mockDriver struct { | |||
func (d *mockDriver) Handles(imageType string) bool { | |||
return d.shouldHandle | |||
} | |||
func (d *mockDriver) Run(op *driver.Operation) (driver.OperationResult, error) { | |||
func (d *mockDriver) Run(context context.Context, op *driver.Operation) (driver.OperationResult, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid shadowing the package name
func (d *mockDriver) Run(context context.Context, op *driver.Operation) (driver.OperationResult, error) { | |
func (d *mockDriver) Run(ctx context.Context, op *driver.Operation) (driver.OperationResult, error) { |
Closing because this is really out of date |
When we execute a bundle operation, pass a context so that they can handle cancellations and timeouts.
The kubernetes driver required enough changes that I created #220 to track updating it to actually use cxt.
TODO: