Skip to content

Commit

Permalink
Revert "client: move context timeout setup based on perProviderTimeou…
Browse files Browse the repository at this point in the history
…t up into Open()"

Since this commit breaks tests and its not completely necessary, it being reverted.
This reverts commit b823d79.
  • Loading branch information
joelrebel committed Jun 22, 2023
1 parent 4294713 commit 0793b70
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 13 deletions.
9 changes: 8 additions & 1 deletion bmc/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"sync"
"time"

"github.com/hashicorp/go-multierror"
"github.com/pkg/errors"
Expand All @@ -28,7 +29,7 @@ type connectionProviders struct {
// OpenConnectionFromInterfaces will try all opener interfaces and remove failed ones.
// The reason failed ones need to be removed is so that when other methods are called (like powerstate)
// implementations that have connections wont nil pointer error when their connection fails.
func OpenConnectionFromInterfaces(ctx context.Context, providers []interface{}) (opened []interface{}, metadata Metadata, err error) {
func OpenConnectionFromInterfaces(ctx context.Context, timeout time.Duration, providers []interface{}) (opened []interface{}, metadata Metadata, err error) {
metadata = Metadata{
FailedProviderDetail: make(map[string]string),
}
Expand All @@ -40,6 +41,12 @@ func OpenConnectionFromInterfaces(ctx context.Context, providers []interface{})
default:
}

// Create a context with the specified timeout. This is done for backward compatibility but
// we should consider removing the timeout parameter alltogether given the context will
// container the timeout.
ctx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()

// result facilitates communication of data between the concurrent opener goroutines and
// the the parent goroutine.
type result struct {
Expand Down
2 changes: 1 addition & 1 deletion bmc/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestOpenConnectionFromInterfaces(t *testing.T) {
tc.ctxTimeout = time.Second * 3
}
ctx := context.Background()
opened, metadata, err := OpenConnectionFromInterfaces(ctx, generic)
opened, metadata, err := OpenConnectionFromInterfaces(ctx, tc.ctxTimeout, generic)
if err != nil {
if tc.err != nil {
if diff := cmp.Diff(err.Error(), tc.err.Error()); diff != "" {
Expand Down
12 changes: 1 addition & 11 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,17 +225,7 @@ func (c *Client) registry() *registrar.Registry {
// from the client.Registry.Drivers. If client.Registry.Drivers ends up
// being empty then we error.
func (c *Client) Open(ctx context.Context) error {
//
// TODO: check with Jacob if the comment below still applies,
//
// Create a context with the specified timeout. This is done for backward compatibility but
// we should consider removing the timeout parameter alltogether given the context will
// container the timeout.
//
ctx, cancel := context.WithTimeout(ctx, c.perProviderTimeout(ctx))
defer cancel()

ifs, metadata, err := bmc.OpenConnectionFromInterfaces(ctx, c.registry().GetDriverInterfaces())
ifs, metadata, err := bmc.OpenConnectionFromInterfaces(ctx, c.perProviderTimeout(ctx), c.registry().GetDriverInterfaces())
defer c.setMetadata(metadata)
if err != nil {
return err
Expand Down

0 comments on commit 0793b70

Please sign in to comment.