From e650959b62dfe6f643e75cd9b6ba295fbd94e830 Mon Sep 17 00:00:00 2001 From: Vartan Babayan <v.babayan@picodata.io> Date: Tue, 10 Dec 2024 19:42:30 +0300 Subject: [PATCH] feat: add valid exit code for admin and connect cli --- src/cli/admin.rs | 10 ++++++- src/cli/connect.rs | 18 ++++++++---- test/int/test_cli_ux.py | 65 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 6 deletions(-) diff --git a/src/cli/admin.rs b/src/cli/admin.rs index 61b366ee7d..ae95e416df 100644 --- a/src/cli/admin.rs +++ b/src/cli/admin.rs @@ -5,6 +5,7 @@ use std::rc::Rc; use std::str::from_utf8; use std::time::Duration; +use nix::unistd::isatty; use rustyline::completion::{extract_word, Completer}; use rustyline::error::ReadlineError; use rustyline::Context; @@ -280,6 +281,12 @@ fn admin_repl(args: args::Admin) -> Result<(), ReplError> { Command::Expression(line) => { temp_client.write(&line)?; let raw_response = temp_client.read()?; + + let is_terminal = isatty(0).unwrap_or(false); + if !is_terminal && raw_response.contains("parsing error") { + return Err(ReplError::Other(raw_response)); + } + let formatted = match temp_client.current_language { ConsoleLanguage::Lua => raw_response, ConsoleLanguage::Sql => serde_yaml::from_str::<ResultSet>(&raw_response) @@ -290,6 +297,7 @@ fn admin_repl(args: args::Admin) -> Result<(), ReplError> { )) })? .to_string(), + }; console.write(&formatted); @@ -304,7 +312,7 @@ pub fn main(args: args::Admin) -> ! { let tt_args = args.tt_args().unwrap(); super::tarantool::main_cb(&tt_args, || -> Result<(), ReplError> { if let Err(err) = admin_repl(args) { - println!("{}", err); + eprintln!("{}", err); std::process::exit(1); } std::process::exit(0) diff --git a/src/cli/connect.rs b/src/cli/connect.rs index 630911c34a..fe8b9a24c6 100644 --- a/src/cli/connect.rs +++ b/src/cli/connect.rs @@ -8,6 +8,7 @@ use crate::address::IprotoAddress; use crate::config::DEFAULT_USERNAME; use crate::traft::error::Error; use crate::util::prompt_password; +use nix::unistd::isatty; use super::args; use super::console::{Command, Console, ReplError, SpecialCommand}; @@ -282,12 +283,19 @@ fn sql_repl(args: args::Connect) -> Result<(), ReplError> { } Err(err) => match err { - tarantool::network::ClientError::ErrorResponse(err) => err.to_string(), + tarantool::network::ClientError::ErrorResponse(err) => { + let is_terminal = isatty(0).unwrap_or(false); + if !is_terminal { + return Err(ReplError::Other(err.to_string())); + } + + err.to_string() + } tarantool::network::ClientError::ConnectionClosed(_) => { return Err(ReplError::Other( - "Server closed the connection unexpectedly. Try to reconnect." - .into(), - )) + "Server unexpectedly closed the connection. Try to reconnect." + .to_string(), + )); } e => return Err(e.into()), }, @@ -305,7 +313,7 @@ pub fn main(args: args::Connect) -> ! { let tt_args = args.tt_args().unwrap(); super::tarantool::main_cb(&tt_args, || -> Result<(), ReplError> { if let Err(error) = sql_repl(args) { - println!("{}", error); + eprintln!("{}", error); std::process::exit(1); } std::process::exit(0) diff --git a/test/int/test_cli_ux.py b/test/int/test_cli_ux.py index b984d15b83..8d2a755f36 100644 --- a/test/int/test_cli_ux.py +++ b/test/int/test_cli_ux.py @@ -696,3 +696,68 @@ def test_picodata_version(cluster: Cluster): assert_starts_with(next(lines), b"picodata ") assert_starts_with(next(lines), b"tarantool (fork) version") assert_starts_with(next(lines), b"target: Linux") + + +def test_admin_cli_exit_code(cluster: Cluster): + setup_sql = f"{cluster.data_dir}/setup.sql" + with open(setup_sql, "w") as f: + f.write( + """ + CREATE USER "alice" WITH PASSWORD 'T0psecret'; + GRANT CREATE TABLE TO "alice"; + GRANT_SYNTAX_ERROR READ TABLE TO "alice"; + GRANT WRITE TABLE TO "alice"; + """ + ) + + i1 = cluster.add_instance(wait_online=False) + i1.start() + i1.wait_online() + + process = subprocess.run( + [i1.binary_path, "admin", f"{i1.data_dir}/admin.sock"], + stdin=open(setup_sql, "r"), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding="utf-8", + timeout=CLI_TIMEOUT, + ) + + assert process.stderr.find("rule parsing error") != -1 + assert process.stderr.find('GRANT_SYNTAX_ERROR READ TABLE TO "alice"') != -1 + assert ( + process.returncode != 0 + ), f"Process failed with exit code {process.returncode}\n" + + +def test_connect_cli_exit_code(cluster: Cluster): + connect_sql = f"{cluster.data_dir}/connect.sql" + with open(connect_sql, "w") as f: + f.write( + """ + CREATE TABLE index (id INTEGER PRIMARY KEY, item TEXT NOT NULL); + SELECT_WITH_SYNTAX_ERROR * FROM index; + SELECT id from index; + """ + ) + + i1 = cluster.add_instance(wait_online=False) + i1.start() + i1.wait_online() + i1.create_user(with_name="andy", with_password="Testpa55") + i1.sql('GRANT CREATE TABLE TO "andy"', sudo=True) + + process = subprocess.run( + [i1.binary_path, "admin", f"{i1.data_dir}/admin.sock"], + stdin=open(connect_sql, "r"), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding="utf-8", + timeout=CLI_TIMEOUT, + ) + + assert process.stderr.find("rule parsing error") != -1 + assert process.stderr.find("SELECT_WITH_SYNTAX_ERROR * FROM index") != -1 + assert ( + process.returncode != 0 + ), f"Process failed with exit code {process.returncode}\n" -- GitLab