From 18462f92a4905b6ca49f3d13b9e8c4121febfd1a Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Tue, 18 Jul 2023 16:17:30 -0700 Subject: [PATCH 1/7] add warning message for unmigrated cli commands --- go/cmd/dolt/commands/sql.go | 8 +++----- go/cmd/dolt/dolt.go | 7 +++++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/go/cmd/dolt/commands/sql.go b/go/cmd/dolt/commands/sql.go index 043a4a71f2..005482939f 100644 --- a/go/cmd/dolt/commands/sql.go +++ b/go/cmd/dolt/commands/sql.go @@ -269,11 +269,9 @@ func sqlHandleVErrAndExitCode(queryist cli.Queryist, verr errhand.VerboseError, if _, ok := queryist.(*engine.SqlEngine); !ok { // We are in a context where we are attempting to connect to a remote database. These errors // are unstructured, so we add some additional context around them. - tmpMsg := `You've encountered a new behavior in dolt which is not fully documented yet. -A local dolt server is using your dolt data directory, and in an attempt to service your request, we are attempting to -connect to it. That has failed. You should stop the server, or reach out to @macneale on https://discord.gg/gqr7K4VNKe -for help.` - cli.PrintErrln(tmpMsg) + remoteUnsupportedMsg := `This command has not yet been migrated to function in a remote context. Please shut down your +server and try again. Or, reach out to us on discord (https://discord.gg/gqr7K4VNKe) if you need help.` + cli.PrintErrln(remoteUnsupportedMsg) msg = fmt.Sprintf("A local server is running, and dolt is failing to connect. Error connecting to remote database: \"%s\".\n", msg) } cli.PrintErrln(msg) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index b9b1a796e4..1cba166db8 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -536,6 +536,13 @@ func runMain() int { cli.PrintErrln(color.RedString("Unexpected Error: %v", err)) return 1 } + } else { + if apr.NArg() > 0 { + cli.PrintErrln(color.RedString(`Global arguments are not supported for this command as it has not yet +been migrated to function in a remote context. Please shut down your server and try again. Or, reach out to us on discord +(https://discord.gg/gqr7K4VNKe) if you need help.`)) + return 1 + } } ctx, stop := context.WithCancel(ctx) From 7f9e128fd25f8134f5b660446eb9ebd1e04084ed Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Tue, 18 Jul 2023 17:12:52 -0700 Subject: [PATCH 2/7] remove unneeded error msg --- go/cmd/dolt/commands/sql.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/go/cmd/dolt/commands/sql.go b/go/cmd/dolt/commands/sql.go index 005482939f..634f61f202 100644 --- a/go/cmd/dolt/commands/sql.go +++ b/go/cmd/dolt/commands/sql.go @@ -266,14 +266,6 @@ func (cmd SqlCmd) Exec(ctx context.Context, commandStr string, args []string, dE func sqlHandleVErrAndExitCode(queryist cli.Queryist, verr errhand.VerboseError, usage cli.UsagePrinter) int { if verr != nil { if msg := verr.Verbose(); strings.TrimSpace(msg) != "" { - if _, ok := queryist.(*engine.SqlEngine); !ok { - // We are in a context where we are attempting to connect to a remote database. These errors - // are unstructured, so we add some additional context around them. - remoteUnsupportedMsg := `This command has not yet been migrated to function in a remote context. Please shut down your -server and try again. Or, reach out to us on discord (https://discord.gg/gqr7K4VNKe) if you need help.` - cli.PrintErrln(remoteUnsupportedMsg) - msg = fmt.Sprintf("A local server is running, and dolt is failing to connect. Error connecting to remote database: \"%s\".\n", msg) - } cli.PrintErrln(msg) } From 9598f4750848bd7dc136ea6a23138ff1d084702b Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Wed, 19 Jul 2023 12:29:47 -0700 Subject: [PATCH 3/7] update logic for detecting unmigrated command --- go/cmd/dolt/dolt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 1cba166db8..22fd052f9a 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -537,7 +537,7 @@ func runMain() int { return 1 } } else { - if apr.NArg() > 0 { + if args[0] != subcommandName { cli.PrintErrln(color.RedString(`Global arguments are not supported for this command as it has not yet been migrated to function in a remote context. Please shut down your server and try again. Or, reach out to us on discord (https://discord.gg/gqr7K4VNKe) if you need help.`)) From 51075bbfed28683aca482b7695c808f542d74261 Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Wed, 19 Jul 2023 12:29:57 -0700 Subject: [PATCH 4/7] add test for unmigrated command error message --- integration-tests/bats/sql-local-remote.bats | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 3ff249dded..019a18effe 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -1087,3 +1087,11 @@ SQL [[ $output =~ "dolt checkout can not currently be used when there is a local server running. Please stop your dolt sql-server and try again." ]] || false } + +@test "sql-local-remote: verify unmigrated command will fail with warning" { + cd altDB + start_sql_server altDB + run dolt --user dolt log + [ $status -eq 1 ] + [[ "$output" =~ "Global arguments are not supported for this command" ]] +} From b08a4296d2573d1d2f4e08211c2416ac2b0d2b3d Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Thu, 20 Jul 2023 10:11:44 -0700 Subject: [PATCH 5/7] add warning for cli commands that don't support global args --- go/cmd/dolt/dolt.go | 30 +++++++++++++++++++- integration-tests/bats/sql-local-remote.bats | 8 ++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 22fd052f9a..9d4502496f 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -152,6 +152,20 @@ var commandsWithoutCliCtx = []cli.Command{ &commands.Assist{}, } +var commandsWithoutGlobalArgSupport = []cli.Command{ + commands.InitCmd{}, + commands.CloneCmd{}, + docscmds.Commands, + commands.MigrateCmd{}, + commands.ReadTablesCmd{}, + commands.LoginCmd{}, + credcmds.Commands, + sqlserver.SqlServerCmd{VersionStr: Version}, + sqlserver.SqlClientCmd{VersionStr: Version}, + commands.VersionCmd{VersionStr: Version}, + commands.ConfigCmd{}, +} + func initCliContext(commandName string) bool { for _, command := range commandsWithoutCliCtx { if command.Name() == commandName { @@ -161,6 +175,15 @@ func initCliContext(commandName string) bool { return true } +func supportsGlobalArgs(commandName string) bool { + for _, command := range commandsWithoutGlobalArgSupport { + if command.Name() == commandName { + return false + } + } + return true +} + var doltCommand = cli.NewSubCommandHandler("dolt", "it's git for data", doltSubCommands) var globalArgParser = buildGlobalArgs() var globalDocs = cli.CommandDocsForCommandString("dolt", doc, globalArgParser) @@ -538,9 +561,14 @@ func runMain() int { } } else { if args[0] != subcommandName { - cli.PrintErrln(color.RedString(`Global arguments are not supported for this command as it has not yet + if supportsGlobalArgs(subcommandName) { + cli.PrintErrln(color.RedString(`Global arguments are not supported for this command as it has not yet been migrated to function in a remote context. Please shut down your server and try again. Or, reach out to us on discord (https://discord.gg/gqr7K4VNKe) if you need help.`)) + } else { + cli.PrintErrln(color.RedString(`This command does not support global arguments. Please try again without +the global arguments or check the docs for questions about usage.`)) + } return 1 } } diff --git a/integration-tests/bats/sql-local-remote.bats b/integration-tests/bats/sql-local-remote.bats index 019a18effe..81c2202a50 100644 --- a/integration-tests/bats/sql-local-remote.bats +++ b/integration-tests/bats/sql-local-remote.bats @@ -1095,3 +1095,11 @@ SQL [ $status -eq 1 ] [[ "$output" =~ "Global arguments are not supported for this command" ]] } + +@test "sql-local-remote: verify commands without global arg support will fail with warning" { + cd altDB + start_sql_server altDB + run dolt --user dolt version + [ $status -eq 1 ] + [[ "$output" =~ "This command does not support global arguments." ]] +} From 6c3fe5ffa4bf1efc980f2143c470707eba674aff Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Thu, 20 Jul 2023 10:13:37 -0700 Subject: [PATCH 6/7] update no-repo test with global arg supporting command --- integration-tests/bats/no-repo.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/bats/no-repo.bats b/integration-tests/bats/no-repo.bats index 58b01c7509..ec98ceb1fb 100755 --- a/integration-tests/bats/no-repo.bats +++ b/integration-tests/bats/no-repo.bats @@ -358,10 +358,10 @@ NOT_VALID_REPO_ERROR="The current directory is not a valid dolt repository." } @test "no-repo: check that we correctly parse args when connecting with a username that matches a subcommand" { - run dolt --user ls version + run dolt --user ls sql -q "select 1" [ "$status" -eq 0 ] - [[ "$output" =~ "version" ]] || false + [[ "$output" =~ "1" ]] || false } @test "no-repo: check that we error on commands with no subcommand" { From 2e9046d5fef6bc27a462cdd5aa3382a953eaba7e Mon Sep 17 00:00:00 2001 From: Stephanie You Date: Thu, 20 Jul 2023 10:26:51 -0700 Subject: [PATCH 7/7] fix formatting for unmigrated command warning --- go/cmd/dolt/dolt.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 9d4502496f..512f0b0003 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -562,12 +562,13 @@ func runMain() int { } else { if args[0] != subcommandName { if supportsGlobalArgs(subcommandName) { - cli.PrintErrln(color.RedString(`Global arguments are not supported for this command as it has not yet -been migrated to function in a remote context. Please shut down your server and try again. Or, reach out to us on discord -(https://discord.gg/gqr7K4VNKe) if you need help.`)) + cli.PrintErrln( + `Global arguments are not supported for this command as it has not yet been migrated to function in a remote context. +If you're interested in running this command against a remote host, hit us up on discord (https://discord.gg/gqr7K4VNKe).`) } else { - cli.PrintErrln(color.RedString(`This command does not support global arguments. Please try again without -the global arguments or check the docs for questions about usage.`)) + cli.PrintErrln( + `This command does not support global arguments. Please try again without the global arguments +or check the docs for questions about usage.`) } return 1 }