cli args for connection options #95
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements significant improvements to the rainfrog CLI, addressing the security concerns raised in Issue #75 regarding password handling. Key changes include:
- New CLI options for individual connection parameters (username, password, host, port, database)
- Flexible input methods, including prompting for missing information
- Use of PgConnectOptions instead of connection strings for improved security
- Updated README with clear instructions on various ways to provide connection details
- Removal of tick_rate and frame_rate options for simplified configuration
These changes enhance the tool's security and usability, providing users with more secure ways to input database credentials.
6 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings
| @@ -29,7 +35,8 @@ async fn tokio_main() -> Result<()> { | |||
| initialize_panic_handler()?; | |||
|
|
|||
| let args = Cli::parse(); | |||
| let mut app = App::new(args.connection_url, args.tick_rate, args.frame_rate)?; | |||
| let connection_opts = build_connection_opts(args.clone())?; | |||
There was a problem hiding this comment.
style: Cloning args here may be unnecessary. Consider passing by reference if possible.
| io::stdout().flush().unwrap(); | ||
| io::stdin().read_line(&mut user).unwrap(); |
There was a problem hiding this comment.
logic: Use of unwrap() here and in other similar places could lead to panics. Consider using ? or proper error handling.
| if let Some(password) = args.password { | ||
| opts = opts.password(&password); | ||
| } else { | ||
| let mut password = rpassword::prompt_password(format!("password for user {}: ", opts.get_username())).unwrap(); |
There was a problem hiding this comment.
logic: Using unwrap() on prompt_password could lead to a panic if there's an error. Consider proper error handling.
| io::stdin().read_line(&mut port).unwrap(); | ||
| port = port.trim().to_string(); | ||
| if !port.is_empty() { | ||
| opts = opts.port(port.parse()?); |
There was a problem hiding this comment.
style: Parsing the port here could fail. The error is propagated, but consider providing a more specific error message.
| @@ -44,3 +51,77 @@ async fn main() -> Result<()> { | |||
| Ok(()) | |||
| } | |||
| } | |||
|
|
|||
| // sqlx defaults to reading from environment variables if no inputs are provided | |||
There was a problem hiding this comment.
style: The build_connection_opts function is quite long. Consider breaking it down into smaller, more manageable functions.
|
Included in release v0.2.4 |
allows many different ways to run and pass in options. also closes #75