PR Feedack addressed

This commit is contained in:
Neil Macneale IV
2023-06-23 09:19:53 -07:00
parent 5e356298ad
commit f32fc4699a
6 changed files with 17 additions and 19 deletions

View File

@@ -24,9 +24,9 @@ import (
)
type UserPassword struct {
Username string
Password string
Unspecified bool // If true, the user and password were not provided by the user.
Username string
Password string
Specified bool // If true, the user and password were provided by the user.
}
const DOLT_ENV_PWD = "DOLT_CLI_PASSWORD"
@@ -52,11 +52,11 @@ func BuildUserPasswordPrompt(parsedArgs *argparser.ArgParseResults) (newParsedAr
if !hasUserId && !hasPassword {
// Common "out of box" behavior.
return newParsedArgs, &UserPassword{Username: "root", Password: "", Unspecified: true}, nil
return newParsedArgs, &UserPassword{Username: "root", Password: "", Specified: false}, nil
}
if hasUserId && hasPassword {
return newParsedArgs, &UserPassword{Username: userId, Password: password, Unspecified: false}, nil
return newParsedArgs, &UserPassword{Username: userId, Password: password, Specified: true}, nil
}
if hasUserId && !hasPassword {
@@ -82,14 +82,14 @@ func BuildUserPasswordPrompt(parsedArgs *argparser.ArgParseResults) (newParsedAr
}
Println()
}
return newParsedArgs, &UserPassword{Username: userId, Password: password, Unspecified: false}, nil
return newParsedArgs, &UserPassword{Username: userId, Password: password, Specified: true}, nil
}
testOverride, hasTestOverride := os.LookupEnv(DOLT_SILENCE_USER_REQ_FOR_TESTING)
if hasTestOverride && testOverride == "Y" {
// Used for BATS testing only. Typical usage will not hit this path, but we have many legacy tests which
// do not provide a user, and the DOLT_ENV_PWD is set to avoid the prompt.
return newParsedArgs, &UserPassword{Unspecified: false}, nil
return newParsedArgs, &UserPassword{Specified: true}, nil
}
return nil, nil, fmt.Errorf("When a password is provided, a user must also be provided. Use the --user flag to provide a username")

View File

@@ -14,7 +14,7 @@
package cli
// Constants for command line flags names. These tend to be used in multiple places, so definding
// Constants for command line flags names. These tend to be used in multiple places, so defining
// them low in the package dependency tree makes sense.
const (
AbortParam = "abort"

View File

@@ -302,7 +302,7 @@ func GetClientConfig(cwdFS filesys.Filesys, creds *cli.UserPassword, apr *argpar
yamlCfg = cfg.(YAMLConfig)
// if command line user argument was given, replace yaml's user and password
if !creds.Unspecified {
if creds.Specified {
yamlCfg.UserConfig.Name = &creds.Username
yamlCfg.UserConfig.Password = &creds.Password
}
@@ -392,7 +392,7 @@ func SetupDoltConfig(dEnv *env.DoltEnv, apr *argparser.ArgParseResults, config S
// getCommandLineConfig sets server config variables and persisted global variables with values defined on command line.
// If not defined, it sets variables to default values. The creds option is available when building a client config, which
// is used for most commands. `dolt sql-server` is special, and it's user/pwd is passed in vie apr, so creds must be nil
// is used for most commands. `dolt sql-server` is special, and its user/pwd is pa`ssed in via apr, so creds must be nil
// to indicate that the user/pwd should be taken from the apr.
func getCommandLineConfig(creds *cli.UserPassword, apr *argparser.ArgParseResults) (ServerConfig, error) {
config := DefaultServerConfig()

View File

@@ -242,7 +242,7 @@ func newLateBindingEngine(
}
var dbUser string
if !creds.Unspecified {
if creds.Specified {
dbUser = creds.Username
// When running in local mode, we want to attempt respect the user/pwd they provided. If they didn't provide

View File

@@ -604,8 +604,8 @@ func buildLateBinder(ctx context.Context, cwdFS filesys.Filesys, mrEnv *env.Mult
cli.Println("verbose: starting remote mode")
}
if creds.Unspecified {
creds = &cli.UserPassword{Username: sqlserver.LocalConnectionUser, Password: lock.Secret, Unspecified: true}
if !creds.Specified {
creds = &cli.UserPassword{Username: sqlserver.LocalConnectionUser, Password: lock.Secret, Specified: false}
}
return sqlserver.BuildConnectionStringQueryist(ctx, cwdFS, creds, apr, lock.Port, useDb)

View File

@@ -293,9 +293,8 @@ func TestDropValue(t *testing.T) {
}
newApr1 := apr.DropValue("string")
if apr.Equals(newApr1) {
t.Error("Original value and new value are equal")
}
require.NotEqualf(t, apr, newApr1, "Original value and new value are equal")
_, hasVal := newApr1.GetValue("string")
if hasVal {
t.Error("DropValue failed to drop string")
@@ -309,9 +308,8 @@ func TestDropValue(t *testing.T) {
}
newApr2 := apr.DropValue("flag")
if apr.Equals(newApr2) {
t.Error("DropValue failed to drop flag")
}
require.NotEqualf(t, apr, newApr2, "DropValue failes to drop flag")
_, hasVal = newApr2.GetValue("string")
if !hasVal {
t.Error("DropValue dropped the wrong value")