Repository: bacalhau-project/bacalhau (language: python)

Issue:
config set is not consistent with other commands
`config set` accepts space separated keys and values, and this behavour is printed first in the `--help` output, but this format is not compatible in `bacalhau serve` and all other commands that accept key/value pair of configurations. 

Either provide support for space separated values everywhere, or remove it from `config set` as well. If we decide to remove it, then we need to provide a clear error message to the user

```
→ bacalhau config set --help
The 'set' command allows you to modify configuration values in your Bacalhau configuration file.

This command supports two input formats for setting configuration values:

1. Key-Value Pair Format:
   You can provide the key and value as separate arguments.
   Example:
     bacalhau config set api.host 127.0.0.1
     bacalhau config set compute.orchestrators http://127.0.0.1:1234 http://1.1.1.1:1234
     bacalhau config set compute.labels foo=bar,baz=buz

2. Key=Value Format:
   Alternatively, you can pass the key and value together in a single argument using the 'key=value' format.
   Example:
     bacalhau config set api.host=127.0.0.1
     bacalhau config set compute.orchestrators=http://127.0.0.1:1234,http://1.1.1.1:1234
     bacalhau config set compute.labels=foo=bar,baz=buz

Usage:
  /Users/walid/bacalhau-versions/v1.5.0/bacalhau config set [flags]
```


Discussion / hints:

Fix patch:
diff --git a/cmd/util/flags/cliflags/config.go b/cmd/util/flags/cliflags/config.go
index d36c60ae56..4773f3e307 100644
--- a/cmd/util/flags/cliflags/config.go
+++ b/cmd/util/flags/cliflags/config.go
@@ -3,15 +3,19 @@ package cliflags
 import (
 	"fmt"
 	"os"
+	"reflect"
 	"strings"
 
 	"github.com/rs/zerolog/log"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 
+	"github.com/bacalhau-project/bacalhau/pkg/bacerrors"
 	"github.com/bacalhau-project/bacalhau/pkg/config/types"
 )
 
+const errComponent = "Config"
+
 // ConfigAutoComplete provides auto-completion suggestions for configuration keys.
 func ConfigAutoComplete(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 	var completions []string
@@ -55,7 +59,9 @@ func (cf *ConfigFlag) Set(value string) error {
 	if cf.isWriteMode {
 		// Check if a config file is already set in Viper
 		if viper.GetString(RootCommandConfigFiles) != "" {
-			return fmt.Errorf("single config file can be set")
+			return bacerrors.New("single config file can be set").
+				WithComponent(errComponent).
+				WithCode(bacerrors.ValidationError)
 		}
 	}
 	cf.Value = value
@@ -75,7 +81,9 @@ func (cf *ConfigFlag) Parse() error {
 
 func (cf *ConfigFlag) parseWriteMode() error {
 	if !cf.isConfigFile() {
-		return fmt.Errorf("config file must end with '.yaml' or '.yml'")
+		return bacerrors.New("config file must end with '.yaml' or '.yml'").
+			WithComponent(errComponent).
+			WithCode(bacerrors.ValidationError)
 	}
 
 	if err := validateFile(cf.Value); err != nil {
@@ -95,14 +103,28 @@ func (cf *ConfigFlag) parseReadMode() error {
 		return cf.addConfigFile()
 	} else {
 		// Handle dot separated path with boolean value
-		return setIfValid(viper.GetViper(), cf.Value, true)
+		key := strings.ToLower(cf.Value)
+		if err := validateKey(key); err != nil {
+			return err
+		}
+		typ := types.AllKeys()[key]
+		if typ.Kind() == reflect.Bool {
+			return setIfValid(viper.GetViper(), cf.Value, true)
+		} else {
+			return bacerrors.New("config flag key requires a value.\n"+
+				"To correct this provide a value to the config, e.g. --config %s=<value>", cf.Value).
+				WithComponent(errComponent).
+				WithCode(bacerrors.ValidationError)
+		}
 	}
 }
 
 func (cf *ConfigFlag) parseKeyValue() error {
 	tokens := strings.SplitN(cf.Value, "=", 2)
 	if len(tokens) != 2 {
-		return fmt.Errorf("config flag value %s is invalid", cf.Value)
+		return bacerrors.New("invalid config flag value: %s", cf.Value).
+			WithComponent(errComponent).
+			WithCode(bacerrors.ValidationError)
 	}
 	return setIfValid(viper.GetViper(), tokens[0], tokens[1])
 }
@@ -122,12 +144,18 @@ func validateFile(filePath string) error {
 	stat, err := os.Stat(filePath)
 	if err != nil {
 		if os.IsNotExist(err) {
-			return fmt.Errorf("the specified configuration file %q doesn't exist", filePath)
+			return bacerrors.New("specified configuration file doesn't exist: %s", filePath).
+				WithComponent(errComponent).
+				WithCode(bacerrors.NotFoundError)
 		}
-		return fmt.Errorf("the specified configuration file %q cannot be read: %w", filePath, err)
+		return bacerrors.Wrap(err, "specified configuration file cannot be read: %s", filePath).
+			WithComponent(errComponent).
+			WithCode(bacerrors.IOError)
 	}
 	if stat.IsDir() {
-		return fmt.Errorf("the specified configuration file %q is a directory, must be a file", filePath)
+		return bacerrors.New("specified configuration file is a directory, must be a file: %s", filePath).
+			WithComponent(errComponent).
+			WithCode(bacerrors.ValidationError)
 	}
 	if stat.Size() == 0 {
 		log.Warn().Msgf("the specified configuration file is empty and ineffectual")
@@ -135,18 +163,31 @@ func validateFile(filePath string) error {
 	return nil
 }
 
-func setIfValid(v *viper.Viper, key string, value any) error {
+func validateKey(key string) error {
 	key = strings.ToLower(key)
 	_, ok := types.AllKeys()[key]
 	if !ok {
 		if _, err := os.Stat(key); err == nil {
-			return fmt.Errorf("config files must end in suffix '.yaml' or '.yml'")
+			return bacerrors.New("config files must end in suffix '.yaml' or '.yml'").
+				WithCode(bacerrors.ValidationError).
+				WithComponent(errComponent)
 		}
-		return fmt.Errorf("no config key matching %q run 'bacalhau config list' for a list of valid keys", key)
+		return bacerrors.New("no matching config key. Run '%s config list' for a list of valid keys", os.Args[0]).
+			WithCode(bacerrors.ValidationError).
+			WithComponent(errComponent)
+	}
+	return nil
+}
+
+func setIfValid(v *viper.Viper, key string, value any) error {
+	if err := validateKey(key); err != nil {
+		return err
 	}
 	parsed, err := types.CastConfigValueForKey(key, value)
 	if err != nil {
-		return err
+		return bacerrors.Wrap(err, "failed to cast config value for key: %s, value: %v", key, value).
+			WithComponent(errComponent).
+			WithCode(bacerrors.ValidationError)
 	}
 	configMap := v.GetStringMap(RootCommandConfigValues)
 	configMap[key] = parsed
diff --git a/pkg/bacerrors/codes.go b/pkg/bacerrors/codes.go
index 498c5077e5..cf52c598c0 100644
--- a/pkg/bacerrors/codes.go
+++ b/pkg/bacerrors/codes.go
@@ -19,6 +19,7 @@ const (
 	ConfigurationError ErrorCode = "ConfigurationError"
 	DatastoreFailure   ErrorCode = "DatastoreFailure"
 	RequestCancelled   ErrorCode = "RequestCancelled"
+	IOError            ErrorCode = "IOError"
 	UnknownError       ErrorCode = "UnknownError"
 )
 
diff --git a/pkg/config/config.go b/pkg/config/config.go
index b8b702cd7b..5d110c761d 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -13,6 +13,7 @@ import (
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 
+	"github.com/bacalhau-project/bacalhau/pkg/bacerrors"
 	"github.com/bacalhau-project/bacalhau/pkg/config/types"
 	"github.com/bacalhau-project/bacalhau/pkg/util/idgen"
 )
@@ -141,6 +142,10 @@ func New(opts ...Option) (*Config, error) {
 		}
 	}
 
+	if err = checkFlagConfigConflicts(c.flags, c.values); err != nil {
+		return nil, err
+	}
+
 	for name, flags := range c.flags {
 		for _, flag := range flags {
 			// only if the flag has been set do we want to bind to it, this allows multiple flags
@@ -311,3 +316,19 @@ func GenerateNodeID(ctx context.Context, nodeNameProviderType string) (string, e
 
 	return nodeName, nil
 }
+
+// checkFlagConfigConflicts checks for conflicts between cli flags and config values.
+// e.g. bacalhau serve --config=api.host=0.0.0.0 --api-host=0.0.0.0 should be rejected.
+func checkFlagConfigConflicts(flags map[string][]*pflag.Flag, cfgValues map[string]any) error {
+	for name, flagList := range flags {
+		if cfgValue, exists := cfgValues[name]; exists {
+			for _, flag := range flagList {
+				if flag.Changed {
+					return bacerrors.New("flag: --%s and config flag key %q cannot both be provided. Only one may be used", flag.Name, name).
+						WithHint("Remove --%s or --config/-c %s=%v from the command", flag.Name, name, cfgValue)
+				}
+			}
+		}
+	}
+	return nil
+}


Test patch:
diff --git a/cmd/util/flags/cliflags/config_test.go b/cmd/util/flags/cliflags/config_test.go
index 36a44d8f6c..83cc2d52d5 100644
--- a/cmd/util/flags/cliflags/config_test.go
+++ b/cmd/util/flags/cliflags/config_test.go
@@ -166,7 +166,7 @@ API:
 	})
 
 	// Test case 4: Dot notation without value (boolean true)
-	t.Run("DotNotationWithoutValue", func(t *testing.T) {
+	t.Run("DotNotationBoolWithoutValue", func(t *testing.T) {
 		viper.Reset()
 		cmd := setupTestCommand(false)
 		cmd.SetArgs([]string{"-c", "WebUI.Enabled"})
@@ -178,6 +178,28 @@ API:
 		assert.True(t, config.WebUI.Enabled)
 	})
 
+	// Test case 4.1: Dot notation with value (boolean true)
+	t.Run("DotNotationBoolWithValue", func(t *testing.T) {
+		viper.Reset()
+		cmd := setupTestCommand(false)
+		cmd.SetArgs([]string{"-c", "WebUI.Enabled=true"})
+		err := cmd.Execute()
+		require.NoError(t, err)
+
+		config, err := loadConfig()
+		require.NoError(t, err)
+		assert.True(t, config.WebUI.Enabled)
+	})
+
+	// Test case 4.2: Dot notation without value (non boolean)
+	t.Run("DotNotationBoolWithValue", func(t *testing.T) {
+		viper.Reset()
+		cmd := setupTestCommand(false)
+		cmd.SetArgs([]string{"-c", "api.host", "0.0.0.0"})
+		err := cmd.Execute()
+		require.Error(t, err)
+	})
+
 	// Test case 5: Multiple dot notation values
 	t.Run("MultipleDotNotationValues", func(t *testing.T) {
 		viper.Reset()
diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
index e76db3a2de..ca7ce120fb 100644
--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@@ -313,20 +313,6 @@ boolValue: false
 		os.Unsetenv("BACALHAU_BOOL_VALUE")
 	}()
 
-	// Set up flags
-	flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
-	flagSet.String("stringValue", "", "test string value")
-	flagSet.Int("intValue", 0, "test int value")
-	flagSet.Bool("boolValue", false, "test bool value")
-
-	err = flagSet.Parse([]string{"--stringValue=from_flag", "--intValue=300", "--boolValue=false"})
-	require.NoError(t, err)
-
-	flags := make(map[string][]*pflag.Flag)
-	flagSet.VisitAll(func(f *pflag.Flag) {
-		flags[f.Name] = []*pflag.Flag{f}
-	})
-
 	// Set up explicit values
 	values := map[string]interface{}{
 		"stringValue": "from_values",
@@ -347,7 +333,6 @@ boolValue: false
 			"intValue":    {"BACALHAU_INT_VALUE"},
 			"boolValue":   {"BACALHAU_BOOL_VALUE"},
 		}),
-		config.WithFlags(flags),
 		config.WithValues(values),
 	)
 	require.NoError(t, err)
@@ -362,6 +347,21 @@ boolValue: false
 	assert.True(t, testCfg.BoolValue, "BoolValue should be overridden by explicit values")
 
 	// Now, let's remove the explicit values and check the precedence of flags
+
+	// Set up flags
+	flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
+	flagSet.String("stringValue", "", "test string value")
+	flagSet.Int("intValue", 0, "test int value")
+	flagSet.Bool("boolValue", false, "test bool value")
+
+	err = flagSet.Parse([]string{"--stringValue=from_flag", "--intValue=300", "--boolValue=false"})
+	require.NoError(t, err)
+
+	flags := make(map[string][]*pflag.Flag)
+	flagSet.VisitAll(func(f *pflag.Flag) {
+		flags[f.Name] = []*pflag.Flag{f}
+	})
+
 	cfg, err = config.New(
 		config.WithDefault(TestConfig{
 			StringValue: "default",
@@ -448,6 +448,89 @@ boolValue: false
 	assert.True(t, testCfg.BoolValue, "BoolValue should be set to default")
 }
 
+func TestFlagConfigConflicts(t *testing.T) {
+	t.Run("No Conflicts", func(t *testing.T) {
+		flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
+		flagSet.String("string-value", "test", "string value")
+		flagSet.Int("int-value", 42, "int value")
+
+		flags := make(map[string][]*pflag.Flag)
+		flagSet.VisitAll(func(f *pflag.Flag) {
+			flags[f.Name] = append(flags[f.Name], f)
+		})
+
+		values := map[string]any{
+			"boolValue": true,
+		}
+
+		cfg, err := config.New(
+			config.WithDefault(TestConfig{}),
+			config.WithFlags(flags),
+			config.WithValues(values),
+		)
+
+		assert.NoError(t, err)
+		assert.NotNil(t, cfg)
+	})
+
+	t.Run("With Conflicts", func(t *testing.T) {
+		flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
+		flagSet.String("string-value", "test", "string value")
+		flagSet.Int("int-value", 42, "int value")
+
+		err := flagSet.Set("string-value", "flag-value")
+		require.NoError(t, err)
+
+		flags := make(map[string][]*pflag.Flag)
+		flagSet.VisitAll(func(f *pflag.Flag) {
+			flags[f.Name] = append(flags[f.Name], f)
+		})
+
+		values := map[string]any{
+			"string-value": "conflict",
+			"boolValue":    true,
+		}
+
+		cfg, err := config.New(
+			config.WithDefault(TestConfig{}),
+			config.WithFlags(flags),
+			config.WithValues(values),
+		)
+
+		assert.Error(t, err)
+		assert.Nil(t, cfg)
+		assert.Contains(t, err.Error(), "flag: --string-value and config flag key \"string-value\" cannot both be provided")
+	})
+
+	t.Run("Multiple Flags for Same Key", func(t *testing.T) {
+		flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
+		flagSet.String("string-value", "test1", "string value 1")
+		flagSet.String("str-value", "test2", "string value 2")
+
+		err := flagSet.Set("str-value", "flag-value")
+		require.NoError(t, err)
+
+		flags := make(map[string][]*pflag.Flag)
+		flagSet.VisitAll(func(f *pflag.Flag) {
+			flags["stringValue"] = append(flags["stringValue"], f)
+		})
+
+		values := map[string]any{
+			"stringValue": "conflict",
+		}
+
+		cfg, err := config.New(
+			config.WithDefault(TestConfig{}),
+			config.WithFlags(flags),
+			config.WithValues(values),
+		)
+
+		assert.Error(t, err)
+		assert.Nil(t, cfg)
+		assert.Contains(t, err.Error(), "flag: --str-value and config flag key \"stringValue\" cannot both be provided")
+	})
+}
+
 type LargeConfig struct {
 	Database struct {
 		Host     string