-
Notifications
You must be signed in to change notification settings - Fork 1
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
timeoutSeconds config in .tbrc.yml #327
Changes from 6 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ import ( | |
"os" | ||
"path/filepath" | ||
"strings" | ||
"time" | ||
|
||
"github.com/TouchBistro/goutils/color" | ||
"github.com/TouchBistro/goutils/errors" | ||
|
@@ -45,6 +46,7 @@ type Config struct { | |
Playlists map[string]playlist.Playlist `yaml:"playlists"` | ||
Overrides map[string]service.ServiceOverride `yaml:"overrides"` | ||
Registries []registry.Registry `yaml:"registries"` | ||
TimeoutSeconds int `yaml:"timeoutSeconds"` | ||
} | ||
|
||
// NOTE: This is deprecated and is only here for backwards compatibility. | ||
|
@@ -151,6 +153,19 @@ func Init(ctx context.Context, config Config, opts InitOptions) (*engine.Engine, | |
return nil, errors.New(errkind.Invalid, "no registries defined", op) | ||
} | ||
|
||
|
||
if config.TimeoutSeconds != 0 && (config.TimeoutSeconds < 5 || config.TimeoutSeconds > 3600) { | ||
return nil, errors.New(errkind.Invalid, fmt.Sprintf("Invalid timeoutSeconds value '%d' in .tbrc.yaml. Values must be between 5 and 3600 inclusive", config.TimeoutSeconds), op) | ||
} | ||
|
||
var timeout time.Duration | ||
if config.TimeoutSeconds != 0 { | ||
timeout = time.Duration(config.TimeoutSeconds) * time.Second | ||
} else { | ||
// default to 60 min timeout when not provided in .tbrc.yml | ||
timeout = time.Duration(3600) * time.Second | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit, could simplify this a bit by assigning the default along with the declaration and eliminate the else case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good, thanks for the suggestion. looks simpler now |
||
|
||
// Validate and normalize all registries. | ||
tracker := progress.TrackerFromContext(ctx) | ||
for i, r := range config.Registries { | ||
|
@@ -184,6 +199,7 @@ func Init(ctx context.Context, config Config, opts InitOptions) (*engine.Engine, | |
err = progress.RunParallel(ctx, progress.RunParallelOptions{ | ||
Message: "Cloning/updating registries", | ||
Count: len(config.Registries), | ||
Timeout: timeout, | ||
}, func(ctx context.Context, i int) error { | ||
r := config.Registries[i] | ||
if r.LocalPath != "" { | ||
|
@@ -293,6 +309,7 @@ func Init(ctx context.Context, config Config, opts InitOptions) (*engine.Engine, | |
BaseImages: registryResult.BaseImages, | ||
LoginStrategies: registryResult.LoginStrategies, | ||
DeviceList: deviceList, | ||
Timeout: timeout, | ||
}) | ||
if err != nil { | ||
return nil, errors.Wrap(err, errors.Meta{Reason: "failed to initialize engine", Op: 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.
Nit, you don't need to explicitly convert numeric constants, i.e.
3600 * time.Second
works