Skip to content

Commit 7da6a82

Browse files
dk19ydk19y
committed
Address PR Feedback
1 parent b89857b commit 7da6a82

File tree

6 files changed

+94
-55
lines changed

6 files changed

+94
-55
lines changed

‎crates/chat-cli/src/api_client/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ impl ApiClient {
138138
model_cache: Arc::new(RwLock::new(None)),
139139
};
140140

141-
if let Some(json) = crate::util::env_var::get_mock_chat_response(Some(env)) {
141+
if let Some(json) = crate::util::env_var::get_mock_chat_response(env) {
142142
this.set_mock_output(serde_json::from_str(fs.read_to_string(json).await.unwrap().as_str()).unwrap());
143143
}
144144

@@ -148,7 +148,7 @@ impl ApiClient {
148148
// If SIGV4_AUTH_ENABLED is true, use Q developer client
149149
let mut streaming_client = None;
150150
let mut sigv4_streaming_client = None;
151-
match crate::util::env_var::is_sigv4_enabled(Some(env)) {
151+
match crate::util::env_var::is_sigv4_enabled(env) {
152152
true => {
153153
let credentials_chain = CredentialsChain::new().await;
154154
if let Err(err) = credentials_chain.provide_credentials().await {

‎crates/chat-cli/src/auth/builder_id.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ use crate::database::{
6363
Database,
6464
Secret,
6565
};
66+
use crate::os::Env;
6667
use crate::util::env_var::is_sigv4_enabled;
6768

6869
#[derive(Debug, Copy, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
@@ -590,7 +591,7 @@ pub async fn poll_create_token(
590591

591592
pub async fn is_logged_in(database: &mut Database) -> bool {
592593
// Check for BuilderId if not using Sigv4
593-
if is_sigv4_enabled(None) {
594+
if is_sigv4_enabled(&Env::new()) {
594595
debug!("logged in using sigv4 credentials");
595596
return true;
596597
}

‎crates/chat-cli/src/logging.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ pub fn get_log_level() -> String {
196196
.lock()
197197
.unwrap()
198198
.clone()
199-
.unwrap_or_else(|| get_env_log_level().unwrap_or_else(|_| DEFAULT_FILTER.to_string()))
199+
.unwrap_or_else(|| get_env_log_level(&crate::os::Env::new()).unwrap_or_else(|_| DEFAULT_FILTER.to_string()))
200200
}
201201

202202
/// Set the log level to the given level.
@@ -247,7 +247,7 @@ fn create_filter_layer() -> EnvFilter {
247247
.lock()
248248
.unwrap()
249249
.clone()
250-
.or_else(|| get_env_log_level().ok());
250+
.or_else(|| get_env_log_level(&crate::os::Env::new()).ok());
251251

252252
match log_level {
253253
Some(level) => EnvFilter::builder()

‎crates/chat-cli/src/telemetry/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ impl TelemetryClient {
540540
return Ok(uuid!("ffffffff-ffff-ffff-ffff-ffffffffffff"));
541541
}
542542

543-
if let Ok(client_id) = crate::util::env_var::get_telemetry_client_id(Some(env)) {
543+
if let Ok(client_id) = crate::util::env_var::get_telemetry_client_id(env) {
544544
if let Ok(uuid) = Uuid::from_str(&client_id) {
545545
return Ok(uuid);
546546
}
@@ -565,7 +565,7 @@ impl TelemetryClient {
565565
}
566566

567567
// cw telemetry is only available with bearer token auth.
568-
let codewhisperer_client = if crate::util::env_var::is_sigv4_enabled(None) {
568+
let codewhisperer_client = if crate::util::env_var::is_sigv4_enabled(&Env::new()) {
569569
None
570570
} else {
571571
Some(ApiClient::new(env, fs, database, None).await?)

‎crates/chat-cli/src/util/consts.rs‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,52 @@ pub mod env_var {
6767
Q_BUNDLE_METADATA_PATH = "Q_BUNDLE_METADATA_PATH",
6868

6969
/// Identifier for the client application or service using the chat-cli
70-
Q_CLI_CLIENT_APPLICATION = "Q_CLI_CLIENT_APPLICATION"
70+
Q_CLI_CLIENT_APPLICATION = "Q_CLI_CLIENT_APPLICATION",
71+
72+
/// Enable logging to stdout
73+
Q_LOG_STDOUT = "Q_LOG_STDOUT",
74+
75+
/// Disable telemetry collection
76+
Q_DISABLE_TELEMETRY = "Q_DISABLE_TELEMETRY",
77+
78+
/// Mock chat response for testing
79+
Q_MOCK_CHAT_RESPONSE = "Q_MOCK_CHAT_RESPONSE",
80+
81+
/// Disable truecolor terminal support
82+
Q_DISABLE_TRUECOLOR = "Q_DISABLE_TRUECOLOR",
83+
84+
/// Fake remote environment for testing
85+
Q_FAKE_IS_REMOTE = "Q_FAKE_IS_REMOTE",
86+
87+
/// Codespaces environment indicator
88+
Q_CODESPACES = "Q_CODESPACES",
89+
90+
/// CI environment indicator
91+
Q_CI = "Q_CI",
92+
93+
/// Telemetry client ID
94+
Q_TELEMETRY_CLIENT_ID = "Q_TELEMETRY_CLIENT_ID",
95+
96+
/// Amazon Q SigV4 authentication
97+
AMAZON_Q_SIGV4 = "AMAZON_Q_SIGV4",
98+
99+
/// Amazon Q chat shell
100+
AMAZON_Q_CHAT_SHELL = "AMAZON_Q_CHAT_SHELL",
101+
102+
/// Editor environment variable
103+
EDITOR = "EDITOR",
104+
105+
/// Terminal type
106+
TERM = "TERM",
107+
108+
/// AWS region
109+
AWS_REGION = "AWS_REGION",
110+
111+
/// GitHub Codespaces environment
112+
CODESPACES = "CODESPACES",
113+
114+
/// CI environment
115+
CI = "CI"
71116
}
72117
}
73118

‎crates/chat-cli/src/util/env_var.rs‎

Lines changed: 40 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,97 +1,90 @@
1-
use std::ffi::OsString;
2-
31
use crate::os::Env;
2+
use crate::util::consts::env_var::*;
43

5-
/// Get environment variable as String
6-
pub fn get_var(key: &str) -> Result<String, std::env::VarError> {
7-
std::env::var(key)
8-
}
9-
10-
/// Get environment variable as OsString
11-
pub fn get_var_os(key: &str) -> Option<OsString> {
12-
std::env::var_os(key)
13-
}
14-
15-
/// Get environment variable with default value
16-
pub fn get_var_or(key: &str, default: &str) -> String {
17-
std::env::var(key).unwrap_or_else(|_| default.to_string())
18-
}
19-
20-
// Semantic helpers that abstract away the actual variable names
21-
pub fn get_log_level() -> Result<String, std::env::VarError> {
22-
get_var("Q_LOG_LEVEL")
4+
/// Get log level from environment
5+
pub fn get_log_level(env: &Env) -> Result<String, std::env::VarError> {
6+
env.get(Q_LOG_LEVEL)
237
}
248

9+
/// Get chat shell with default fallback
2510
#[cfg(unix)]
2611
pub fn get_chat_shell() -> String {
27-
get_var_or("AMAZON_Q_CHAT_SHELL", "bash")
12+
Env::new().get(AMAZON_Q_CHAT_SHELL).unwrap_or_else(|_| "bash".to_string())
2813
}
2914

15+
/// Check if stdout logging is enabled
3016
pub fn is_log_stdout_enabled() -> bool {
31-
get_var_os("Q_LOG_STDOUT").is_some()
17+
Env::new().get_os(Q_LOG_STDOUT).is_some()
3218
}
3319

20+
/// Check if telemetry is disabled
3421
pub fn is_telemetry_disabled() -> bool {
35-
get_var_os("Q_DISABLE_TELEMETRY").is_some()
22+
Env::new().get_os(Q_DISABLE_TELEMETRY).is_some()
3623
}
3724

38-
pub fn get_mock_chat_response(env: Option<&Env>) -> Option<String> {
39-
match env {
40-
Some(e) => e.get("Q_MOCK_CHAT_RESPONSE").ok(),
41-
None => get_var("Q_MOCK_CHAT_RESPONSE").ok(),
42-
}
25+
/// Get mock chat response for testing
26+
pub fn get_mock_chat_response(env: &Env) -> Option<String> {
27+
env.get(Q_MOCK_CHAT_RESPONSE).ok()
4328
}
4429

30+
/// Check if truecolor is disabled
4531
pub fn is_truecolor_disabled() -> bool {
46-
get_var_os("Q_DISABLE_TRUECOLOR").is_some_and(|s| !s.is_empty())
32+
Env::new().get_os(Q_DISABLE_TRUECOLOR).is_some_and(|s| !s.is_empty())
4733
}
4834

35+
/// Check if remote environment is faked
4936
pub fn is_remote_fake() -> bool {
50-
get_var_os("Q_FAKE_IS_REMOTE").is_some()
37+
Env::new().get_os(Q_FAKE_IS_REMOTE).is_some()
5138
}
5239

40+
/// Check if running in Codespaces
5341
pub fn in_codespaces() -> bool {
54-
get_var_os("CODESPACES").is_some() || get_var_os("Q_CODESPACES").is_some()
42+
let env = Env::new();
43+
env.get_os(CODESPACES).is_some() || env.get_os(Q_CODESPACES).is_some()
5544
}
5645

46+
/// Check if running in CI
5747
pub fn in_ci() -> bool {
58-
get_var_os("CI").is_some() || get_var_os("Q_CI").is_some()
48+
let env = Env::new();
49+
env.get_os(CI).is_some() || env.get_os(Q_CI).is_some()
5950
}
6051

52+
/// Get CLI client application
6153
pub fn get_cli_client_application() -> Option<String> {
62-
get_var("Q_CLI_CLIENT_APPLICATION").ok()
54+
Env::new().get(Q_CLI_CLIENT_APPLICATION).ok()
6355
}
6456

57+
/// Get editor with default fallback
6558
pub fn get_editor() -> String {
66-
get_var_or("EDITOR", "vi")
59+
Env::new().get(EDITOR).unwrap_or_else(|_| "vi".to_string())
6760
}
6861

62+
/// Try to get editor without fallback
6963
pub fn try_get_editor() -> Result<String, std::env::VarError> {
70-
get_var("EDITOR")
64+
Env::new().get(EDITOR)
7165
}
7266

67+
/// Get terminal type
7368
pub fn get_term() -> Option<String> {
74-
get_var("TERM").ok()
69+
Env::new().get(TERM).ok()
7570
}
7671

72+
/// Get AWS region
7773
pub fn get_aws_region() -> Result<String, std::env::VarError> {
78-
get_var("AWS_REGION")
74+
Env::new().get(AWS_REGION)
7975
}
8076

81-
pub fn is_sigv4_enabled(env: Option<&Env>) -> bool {
82-
match env {
83-
Some(e) => e.get("AMAZON_Q_SIGV4").is_ok_and(|v| !v.is_empty()),
84-
None => get_var("AMAZON_Q_SIGV4").is_ok_and(|v| !v.is_empty()),
85-
}
77+
/// Check if SigV4 authentication is enabled
78+
pub fn is_sigv4_enabled(env: &Env) -> bool {
79+
env.get(AMAZON_Q_SIGV4).is_ok_and(|v| !v.is_empty())
8680
}
8781

82+
/// Get all environment variables
8883
pub fn get_all_env_vars() -> std::env::Vars {
8984
std::env::vars()
9085
}
9186

92-
pub fn get_telemetry_client_id(env: Option<&Env>) -> Result<String, std::env::VarError> {
93-
match env {
94-
Some(e) => e.get("Q_TELEMETRY_CLIENT_ID"),
95-
None => get_var("Q_TELEMETRY_CLIENT_ID"),
96-
}
87+
/// Get telemetry client ID
88+
pub fn get_telemetry_client_id(env: &Env) -> Result<String, std::env::VarError> {
89+
env.get(Q_TELEMETRY_CLIENT_ID)
9790
}

0 commit comments

Comments
 (0)