Skip to content

Commit 37a629e

Browse files
authored
refactor: improve MySQL credential setup and error handling (#12)
* refactor: improve MySQL credential setup and error handling * ci: fix cargo fmt check
1 parent ca9a8e5 commit 37a629e

File tree

3 files changed

+71
-61
lines changed

3 files changed

+71
-61
lines changed

src/lib.rs

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -36,37 +36,27 @@ pub fn run_cli() -> Result<()> {
3636
.default(true)
3737
.interact()?
3838
{
39-
let mut success = false;
40-
for _ in 0..3 {
41-
match cred_mgr.prompt_credentials_with_connection_test() {
42-
Ok((user, password)) => {
43-
let mysql_config = cred_mgr.encrypt_credentials(&user, &password)?;
44-
doris_config.mysql = Some(mysql_config);
45-
persist_configuration(&doris_config);
46-
47-
match tools::mysql::MySQLTool.query_cluster_info(&doris_config) {
48-
Ok(cluster_info) => {
49-
if let Err(e) = cluster_info.save_to_file() {
50-
ui::print_warning(&format!("Failed to save cluster info: {e}"));
51-
}
52-
}
53-
Err(e) => {
54-
ui::print_warning(&format!("Failed to collect cluster info: {e}"));
39+
match cred_mgr.prompt_credentials_with_connection_test() {
40+
Ok((user, password)) => {
41+
let mysql_config = cred_mgr.encrypt_credentials(&user, &password)?;
42+
doris_config.mysql = Some(mysql_config);
43+
persist_configuration(&doris_config);
44+
45+
match tools::mysql::MySQLTool.query_cluster_info(&doris_config) {
46+
Ok(cluster_info) => {
47+
if let Err(e) = cluster_info.save_to_file() {
48+
ui::print_warning(&format!("Failed to save cluster info: {e}"));
5549
}
5650
}
57-
58-
success = true;
59-
break;
60-
}
61-
Err(e) => {
62-
ui::print_warning(&format!("MySQL credential setup failed: {e}"));
51+
Err(e) => {
52+
ui::print_warning(&format!("Failed to collect cluster info: {e}"));
53+
}
6354
}
6455
}
65-
}
66-
if !success {
67-
ui::print_warning(
68-
"MySQL credential setup failed after 3 attempts. You can configure it later in the settings.",
69-
);
56+
Err(e) => {
57+
ui::print_warning(&format!("MySQL credential setup failed: {e}"));
58+
ui::print_warning("You can configure it later in the settings.");
59+
}
7060
}
7161
}
7262

src/tools/mysql/client.rs

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ impl MySQLTool {
5252
let stderr = String::from_utf8_lossy(&output.stderr);
5353
if stderr.contains("Access denied for user") || stderr.contains("ERROR 1045") {
5454
return Err(CliError::MySQLAccessDenied(stderr.to_string()));
55+
} else {
56+
return Err(CliError::ToolExecutionFailed(format!(
57+
"mysql query failed: {stderr}"
58+
)));
5559
}
5660
}
5761
Ok(String::from_utf8_lossy(&output.stdout).to_string())
@@ -66,19 +70,23 @@ impl MySQLTool {
6670
query: &str,
6771
) -> Result<std::process::Output> {
6872
let mut command = Command::new("mysql");
69-
command.args([
70-
"-h",
71-
host,
72-
"-P",
73-
&port.to_string(),
74-
"-u",
75-
user,
76-
&format!("-p{password}"),
77-
"-A",
78-
"-e",
79-
query,
80-
]);
81-
crate::executor::execute_command(&mut command, "mysql")
73+
command.arg("-h").arg(host);
74+
command.arg("-P").arg(port.to_string());
75+
command.arg("-u").arg(user);
76+
77+
if !password.is_empty() {
78+
command.arg(format!("-p{password}"));
79+
}
80+
81+
command.arg("-A");
82+
command.arg("-e").arg(query);
83+
84+
// Prevent mysql from prompting for a password interactively
85+
command.stdin(std::process::Stdio::null());
86+
87+
command
88+
.output()
89+
.map_err(|e| CliError::ToolExecutionFailed(format!("Failed to execute mysql: {e}")))
8290
}
8391

8492
/// Gets the connection parameters for MySQL, with a clear priority:

src/tools/mysql/credentials.rs

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,34 +39,46 @@ impl CredentialManager {
3939
Ok((user, password))
4040
}
4141

42+
/// Prompts for credentials and verifies them, handling passwordless scenarios.
4243
pub fn prompt_credentials_with_connection_test(&self) -> Result<(String, String)> {
4344
let max_retries = 3;
44-
for attempt in 0..max_retries {
45+
for _ in 0..max_retries {
4546
let (user, password) = self.prompt_for_credentials()?;
46-
// Build a temporary DorisConfig for connection test
47-
let config = DorisConfig {
48-
mysql: Some(MySQLConfig {
49-
user: user.clone(),
50-
password: self.encrypt_password(&password)?,
51-
}),
52-
..Default::default()
53-
};
54-
// Try to run a simple query to test credentials
55-
let test_result = MySQLTool::query_sql_with_config(&config, "SELECT 1");
56-
match test_result {
57-
Ok(_) => return Ok((user, password)),
47+
48+
match self.test_connection(&user, &password) {
49+
Ok(_) => {
50+
if password.is_empty() || self.test_connection(&user, "").is_err() {
51+
return Ok((user, password));
52+
} else {
53+
return Ok((user, "".to_string()));
54+
}
55+
}
5856
Err(CliError::MySQLAccessDenied(_)) => {
59-
println!("Access denied for user. Please try again.");
57+
crate::ui::print_error(
58+
"Access denied. Please check your password and try again.",
59+
);
60+
}
61+
Err(e) => {
62+
return Err(e);
6063
}
61-
Err(e) => return Err(e),
62-
}
63-
if attempt == max_retries - 1 {
64-
return Err(CliError::MySQLAccessDenied(
65-
"Maximum retries reached".to_string(),
66-
));
6764
}
6865
}
69-
unreachable!()
66+
67+
Err(CliError::MySQLAccessDenied(
68+
"Maximum retries reached".to_string(),
69+
))
70+
}
71+
72+
/// Helper function to test a MySQL connection with specific credentials.
73+
fn test_connection(&self, user: &str, password: &str) -> Result<()> {
74+
let config = DorisConfig {
75+
mysql: Some(MySQLConfig {
76+
user: user.to_string(),
77+
password: self.encrypt_password(password)?,
78+
}),
79+
..Default::default()
80+
};
81+
MySQLTool::query_sql_with_config(&config, "SELECT 1").map(|_| ())
7082
}
7183

7284
pub fn encrypt_credentials(&self, user: &str, password: &str) -> Result<MySQLConfig> {

0 commit comments

Comments
 (0)