Skip to content
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

Fix Go lint issues #4647

Merged
merged 7 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion cmd/cli/config/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,12 @@ func newSetCmd() *cobra.Command {
ValidArgsFunction: setAutoComplete,
}

setCmd.PersistentFlags().VarP(cliflags.NewWriteConfigFlag(), "config", "c", "Path to the config file (default is $BACALHAU_DIR/config.yaml)")
setCmd.PersistentFlags().VarP(
cliflags.NewWriteConfigFlag(),
"config",
"c",
"Path to the config file (default is $BACALHAU_DIR/config.yaml)",
)
return setCmd
}

Expand Down
3 changes: 1 addition & 2 deletions cmd/cli/serve/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ func serve(cmd *cobra.Command, cfg types.Bacalhau, fsRepo *repo.FsRepo) error {
if err != nil {
return fmt.Errorf("reloading system metadata after persisting name: %w", err)
}

} else {
// Warn if the flag was provided but node name already exists
if flagNodeName := cmd.PersistentFlags().Lookup(NameFlagName).Value.String(); flagNodeName != "" && flagNodeName != sysmeta.NodeName {
Expand Down Expand Up @@ -248,8 +247,8 @@ func serve(cmd *cobra.Command, cfg types.Bacalhau, fsRepo *repo.FsRepo) error {
if len(cfg.Compute.AllowListedLocalPaths) > 0 {
startupLog.Strs("volumes", cfg.Compute.AllowListedLocalPaths)
}

}

if cfg.Orchestrator.Enabled {
startupLog.Str("orchestrator_address",
fmt.Sprintf("%s:%d", cfg.Orchestrator.Host, cfg.Orchestrator.Port))
Expand Down
1 change: 0 additions & 1 deletion cmd/util/fatal.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ func fatalError(cmd *cobra.Command, err error, code int) {
cmd.PrintErrln(stackTrace)
}
}

} else {
cmd.PrintErrln(output.RedStr("Error: ") + err.Error())
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/util/flags/configflags/deprecation.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import (
"fmt"
)

const FeatureDeprecatedMessage = "This feature has been deprecated and is no longer functional. The flag has no effect and can be safely removed."
const FeatureDeprecatedMessage = "This feature has been deprecated and is no longer " +
"functional. The flag has no effect and can be safely removed."

func makeDeprecationMessage(key string) string {
return fmt.Sprintf("Use %s to set this configuration", makeConfigFlagDeprecationCommand(key))
Expand Down
27 changes: 15 additions & 12 deletions cmd/util/flags/configflags/ipfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,31 @@ var IPFSFlags = []Definition{
),
},
{
FlagName: "ipfs-connect-storage",
ConfigPath: types.InputSourcesTypesIPFSEndpointKey,
DefaultValue: config.Default.InputSources.Types.IPFS.Endpoint,
Description: "The ipfs host multiaddress to connect to for inputs, otherwise an in-process IPFS node will be created if not set.",
FlagName: "ipfs-connect-storage",
ConfigPath: types.InputSourcesTypesIPFSEndpointKey,
DefaultValue: config.Default.InputSources.Types.IPFS.Endpoint,
Description: "The ipfs host multiaddress to connect to for inputs, " +
"otherwise an in-process IPFS node will be created if not set.",
EnvironmentVariables: []string{"BACALHAU_NODE_IPFS_CONNECT"},
Deprecated: true,
DeprecatedMessage: makeDeprecationMessage(types.InputSourcesTypesIPFSEndpointKey),
},
{
FlagName: "ipfs-connect-publisher",
ConfigPath: types.PublishersTypesIPFSEndpointKey,
DefaultValue: config.Default.Publishers.Types.IPFS.Endpoint,
Description: "The ipfs host multiaddress to connect to for publishing, otherwise an in-process IPFS node will be created if not set.",
FlagName: "ipfs-connect-publisher",
ConfigPath: types.PublishersTypesIPFSEndpointKey,
DefaultValue: config.Default.Publishers.Types.IPFS.Endpoint,
Description: "The ipfs host multiaddress to connect to for publishing, " +
"otherwise an in-process IPFS node will be created if not set.",
EnvironmentVariables: []string{"BACALHAU_NODE_IPFS_CONNECT"},
Deprecated: true,
DeprecatedMessage: makeDeprecationMessage(types.PublishersTypesIPFSEndpointKey),
},
{
FlagName: "ipfs-connect-downloader",
ConfigPath: types.ResultDownloadersTypesIPFSEndpointKey,
DefaultValue: config.Default.ResultDownloaders.Types.IPFS.Endpoint,
Description: "The ipfs host multiaddress to connect to for downloading, otherwise an in-process IPFS node will be created if not set.",
FlagName: "ipfs-connect-downloader",
ConfigPath: types.ResultDownloadersTypesIPFSEndpointKey,
DefaultValue: config.Default.ResultDownloaders.Types.IPFS.Endpoint,
Description: "The ipfs host multiaddress to connect to for downloading, " +
"otherwise an in-process IPFS node will be created if not set.",
EnvironmentVariables: []string{"BACALHAU_NODE_IPFS_CONNECT"},
Deprecated: true,
DeprecatedMessage: makeDeprecationMessage(types.ResultDownloadersTypesIPFSEndpointKey),
Expand Down
7 changes: 4 additions & 3 deletions cmd/util/printer/fish_spinner.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import (

// ANSI escape codes for cursor control
const (
hideCursor = "\033[?25l"
showCursor = "\033[?25h"
hideCursor = "\033[?25l"
showCursor = "\033[?25h"
tickerInterval = 200 * time.Millisecond
)

// FishSpinner represents a simple fish emoji spinner.
Expand Down Expand Up @@ -96,7 +97,7 @@ func (s *FishSpinner) Resume() {

// run continuously updates the spinner animation
func (s *FishSpinner) run() {
ticker := time.NewTicker(200 * time.Millisecond)
ticker := time.NewTicker(tickerInterval)
defer ticker.Stop()

for {
Expand Down
12 changes: 9 additions & 3 deletions cmd/util/printer/progress_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ func (j *JobProgressPrinter) followProgress(ctx context.Context, job *models.Job
cmd.Print("Checking job status... (Enter Ctrl+C to exit at any time, your job will continue running):\n\n")
}

eventPrinter := j.createEventPrinter(cmd)
currentEventPrinter := j.createEventPrinter(cmd)

eventChan := make(chan *models.JobHistory)
errChan := make(chan error, 1)
go j.fetchEvents(ctx, job, eventChan, errChan)

// process events until the context is canceled or the job is done
if err := j.handleEvents(ctx, eventPrinter, eventChan, errChan); err != nil {
if err := j.handleEvents(ctx, currentEventPrinter, eventChan, errChan); err != nil {
if errors.Is(err, context.DeadlineExceeded) {
j.printTimeoutMessage(cmd)
return nil
Expand All @@ -125,6 +125,7 @@ func (j *JobProgressPrinter) fetchEvents(ctx context.Context, job *models.Job, e
defer close(eventChan)

// Create a ticker that ticks every 500 milliseconds
//nolint:mnd // Time interval easier to read this way
ticker := time.NewTicker(500 * time.Millisecond)
defer ticker.Stop()

Expand Down Expand Up @@ -163,7 +164,12 @@ func (j *JobProgressPrinter) fetchEvents(ctx context.Context, job *models.Job, e
}
}

func (j *JobProgressPrinter) handleEvents(ctx context.Context, printer eventPrinter, eventChan <-chan *models.JobHistory, errChan <-chan error) error {
func (j *JobProgressPrinter) handleEvents(
ctx context.Context,
printer eventPrinter,
eventChan <-chan *models.JobHistory,
errChan <-chan error,
) error {
defer printer.close() // close the printer to clear any spinner before exiting

for {
Expand Down
15 changes: 14 additions & 1 deletion cmd/util/printer/spinner.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ const (
WidthTimer = 0
)

const (
maxUint8Value = 255
minUint8Value = 0
)

type LineMessage struct {
Message string
Detail string
Expand All @@ -248,10 +253,18 @@ type LineMessage struct {
}

func NewLineMessage(msg string, maxWidth int) LineMessage {
safeWidth := maxWidth
if maxWidth > maxUint8Value {
safeWidth = maxUint8Value // Cap at maximum uint8 value
} else if maxWidth < minUint8Value {
safeWidth = minUint8Value // Handle negative values
}

//nolint:gosec // Safe uint8 conversion - value is bounded by maxUint8Value check above
return LineMessage{
Message: msg,
TimerString: spinnerFmtDuration(SpinnerFormatDurationDefault),
ColumnWidths: []uint8{uint8(maxWidth), WidthDots, WidthStatus, WidthTimer},
ColumnWidths: []uint8{uint8(safeWidth), WidthDots, WidthStatus, WidthTimer},
}
}

Expand Down
2 changes: 2 additions & 0 deletions cmd/util/printer/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const (

var terminalWidth int

//nolint:gosec // terminalWidth is used for spacing and won't exceed reasonable values
func getTerminalWidth(cmd *cobra.Command) uint {
if terminalWidth == 0 {
var err error
Expand Down Expand Up @@ -102,6 +103,7 @@ func SummariseHistoryEvents(history []*models.JobHistory) []models.Event {
return maps.Values(events)
}

//nolint:gosec // indent is used for spacing and won't exceed reasonable values
func printIndentedString(cmd *cobra.Command, prefix, msg string, prefixColor *color.Color, startIndent uint) {
maxWidth := getTerminalWidth(cmd)
blockIndent := int(startIndent) + len(prefix)
Expand Down
2 changes: 1 addition & 1 deletion pkg/bacerrors/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func Wrap(err error, format string, a ...any) Error {
newErr.wrappingMsg = message
return &newErr
}
nErr := New(fmt.Sprintf("%s: %s", message, err.Error()))
nErr := New("%s: %s", message, err.Error())
nErr.(*errorImpl).wrappedErr = err
nErr.(*errorImpl).wrappingMsg = message
return nErr
Expand Down
12 changes: 9 additions & 3 deletions pkg/bacerrors/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import (
"strings"
)

const (
// maxStackDepth defines the maximum number of stack frames to capture
// in the error stack trace.
maxStackDepth = 32
)

// stack represents a stack of program counters.
type stack []uintptr

Expand All @@ -25,9 +31,9 @@ func (s *stack) String() string {
}

func callers() *stack {
const depth = 32
var pcs [depth]uintptr
n := runtime.Callers(4, pcs[:])
const skipCallers = 4
var pcs [maxStackDepth]uintptr
n := runtime.Callers(skipCallers, pcs[:])
var st stack = pcs[0:n]
return &st
}
1 change: 1 addition & 0 deletions pkg/compute/capacity/system/gpu/intel.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func (intel *intelGPUProvider) GetAvailableCapacity(ctx context.Context) (models
// Start with an empty Resources and just fold over it
var allGPUs models.Resources
for _, gpu := range gpuList.GPUs {
//nolint:gosec // G115: GPU indices are always within int range
provider := intel.getInfoProvider(int(gpu.Index))
gpuInfo, err := provider.GetAvailableCapacity(ctx)
if err != nil {
Expand Down
7 changes: 6 additions & 1 deletion pkg/compute/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ func (s BaseEndpoint) GetNodeID() string {
}

func (s BaseEndpoint) AskForBid(ctx context.Context, request AskForBidRequest) (AskForBidResponse, error) {
ctx, span := telemetry.NewSpan(ctx, telemetry.GetTracer(), "pkg/compute.BaseEndpoint.AskForBid", trace.WithSpanKind(trace.SpanKindInternal))
ctx, span := telemetry.NewSpan(
ctx,
telemetry.GetTracer(),
"pkg/compute.BaseEndpoint.AskForBid",
trace.WithSpanKind(trace.SpanKindInternal),
)
defer span.End()
log.Ctx(ctx).Debug().Msgf("asked to bid on: %+v", request)
jobsReceived.Add(ctx, 1)
Expand Down
4 changes: 2 additions & 2 deletions pkg/compute/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type BaseExecutorParams struct {
Store store.ExecutionStore
Storages storage.StorageProvider
StorageDirectory string
Executors executor.ExecutorProvider
Executors executor.ExecProvider
ResultsPath ResultsPath
Publishers publisher.PublisherProvider
FailureInjectionConfig models.FailureInjectionConfig
Expand All @@ -43,7 +43,7 @@ type BaseExecutor struct {
store store.ExecutionStore
Storages storage.StorageProvider
storageDirectory string
executors executor.ExecutorProvider
executors executor.ExecProvider
publishers publisher.PublisherProvider
resultsPath ResultsPath
failureInjection models.FailureInjectionConfig
Expand Down
4 changes: 2 additions & 2 deletions pkg/compute/logstream/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ const defaultBuffer = 100

type ServerParams struct {
ExecutionStore store.ExecutionStore
Executors executor.ExecutorProvider
Executors executor.ExecProvider
// Buffer is the size of the channel buffer for each individual log stream.
// If not set (0), defaultBuffer will be used.
Buffer int
}

type Server struct {
executionStore store.ExecutionStore
executors executor.ExecutorProvider
executors executor.ExecProvider
// buffer is the size of the channel buffer for each individual log stream.
buffer int
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/compute/node_info_decorator.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type NodeInfoDecoratorParams struct {
Executors executor.ExecutorProvider
Executors executor.ExecProvider
Publisher publisher.PublisherProvider
Storages storage.StorageProvider
RunningCapacityTracker capacity.Tracker
Expand All @@ -21,7 +21,7 @@ type NodeInfoDecoratorParams struct {
}

type NodeInfoDecorator struct {
executors executor.ExecutorProvider
executors executor.ExecProvider
publishers publisher.PublisherProvider
storages storage.StorageProvider
runningCapacityTracker capacity.Tracker
Expand Down
7 changes: 6 additions & 1 deletion pkg/compute/store/boltdb/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,12 @@ func (s *Store) GetExecutionCount(ctx context.Context, state store.LocalExecutio
return nil
}

count = uint64(b.Stats().KeyN)
keyCount := b.Stats().KeyN
if keyCount < 0 {
return fmt.Errorf("invalid negative key count from bucket: %d", keyCount)
}

count = uint64(keyCount)
return nil
})

Expand Down
7 changes: 6 additions & 1 deletion pkg/compute/store/boltdb/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,19 @@ import (
bolt "go.etcd.io/bbolt"
)

const (
// uint64ByteSize is the number of bytes needed to represent a uint64
uint64ByteSize = 8
)

// strToBytes converts a string to a byte slice
func strToBytes(s string) []byte {
return []byte(s)
}

// uint64ToBytes converts an uint64 to a byte slice
func uint64ToBytes(i uint64) []byte {
buf := make([]byte, 8) //nolint:gomnd
buf := make([]byte, uint64ByteSize)
binary.BigEndian.PutUint64(buf, i)
return buf
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ func WithValues(values map[string]any) Option {

// New returns a configuration with the provided options applied. If no options are provided, the returned config
// contains only the default values.
//
//nolint:funlen,gocyclo // TODO: This function is very long and complex. Need to improve it.
func New(opts ...Option) (*Config, error) {
base := viper.New()
base.SetEnvPrefix(environmentVariablePrefix)
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/types/downloaders.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type ResultDownloadersTypes struct {

func (r ResultDownloaders) IsNotDisabled(kind string) bool {
return !slices.ContainsFunc(r.Disabled, func(s string) bool {
return strings.ToLower(s) == strings.ToLower(kind)
return strings.EqualFold(s, kind)
})
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/config/types/engines.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type EngineConfigTypes struct {

func (e EngineConfig) IsNotDisabled(kind string) bool {
return !slices.ContainsFunc(e.Disabled, func(s string) bool {
return strings.ToLower(s) == strings.ToLower(kind)
return strings.EqualFold(s, kind)
})
}

Expand Down
9 changes: 8 additions & 1 deletion pkg/config/types/gen/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,14 @@ type FieldInfo struct {
CapitalizedPath string
}

func processStruct(prefix string, capPrefix string, structType *ast.StructType, fieldInfos map[string]FieldInfo, typeMap map[string]*ast.StructType) {
//nolint:funlen,gocyclo // TODO: Function is very long and complex
func processStruct(
prefix string,
capPrefix string,
structType *ast.StructType,
fieldInfos map[string]FieldInfo,
typeMap map[string]*ast.StructType,
) {
for _, field := range structType.Fields.List {
// Get field names
var fieldNames []string
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/types/publishers.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type PublisherTypes struct {

func (p PublishersConfig) IsNotDisabled(kind string) bool {
return !slices.ContainsFunc(p.Disabled, func(s string) bool {
return strings.ToLower(s) == strings.ToLower(kind)
return strings.EqualFold(s, kind)
})
}

Expand Down
Loading
Loading