Mkp/GitHub on prem #74
Conversation
Tr/test author fix
…-fix-condition Test/mkp/bitbucket bug fix condition
…webhook-bug Test/tr/mkp/bitbucket webhook bug
…-fix-condition Test/mkp/bitbucket bug fix condition
…-fix-condition fix response status logic in get pr list
…-fix-condition add debug logs to fix get pr 404 error
…-fix-condition fixed webhook message parsing function
…-fix-condition add debug logs and fix keys
…-fix-condition fix keys in webhook data parsing
Mkp/ablejobs
…-fix-condition Test/mkp/bitbucket bug fix condition
Deploying all bugfixes to ablejobs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| return; | ||
| } | ||
| println!("Processing PR : {}", &review.id()); | ||
| commit_check(&review).await; | ||
| let access_token_opt = get_access_token(&review).await; | ||
| if access_token_opt.is_none(){ | ||
| eprintln!("[process_review] empty access_token_opt"); | ||
| return; | ||
| } | ||
| let access_token = access_token_opt.expect("empty access_token_opt"); | ||
| commit_check(&review, &access_token).await; | ||
| let hunkmap_opt = process_review_changes(&review).await; | ||
| send_hunkmap(&hunkmap_opt, &review, &repo_config).await; | ||
| send_hunkmap(&hunkmap_opt, &review, &repo_config, &access_token).await; | ||
| } |
There was a problem hiding this comment.
The process_review function now includes an asynchronous call to get_access_token and conditional checks for its presence. This is a good practice for handling potential null values. However, the expect calls after is_none() checks are redundant and can be replaced with unwrap_or_else or if let constructs to avoid panicking.
if let Some(access_token) = access_token_opt {
commit_check(&review, &access_token).await;
let hunkmap_opt = process_review_changes(&review).await;
send_hunkmap(&hunkmap_opt, &review, &repo_config, &access_token).await;
} else {
eprintln!("[process_review] empty access_token_opt");
}
| async fn get_access_token (review: &Review) -> Option<String> { | ||
| let access_token: String; | ||
| if review.provider().to_string() == ProviderEnum::Bitbucket.to_string().to_lowercase() { | ||
| let access_token_opt = bitbucket::auth::refresh_git_auth(review.clone_url(), review.clone_dir()).await; | ||
| if access_token_opt.is_none() { | ||
| eprintln!("no refresh token acquired"); | ||
| return None; | ||
| } | ||
| access_token = access_token_opt.expect("Empty access_token_opt"); | ||
| } | ||
| else if review.provider().to_string() == ProviderEnum::Github.to_string().to_lowercase(){ | ||
| let access_token_opt = github::auth::refresh_git_auth(review.clone_url(), review.clone_dir()).await; | ||
| if access_token_opt.is_none() { | ||
| eprintln!("no refresh token acquired"); | ||
| return None; | ||
| } | ||
| access_token = access_token_opt.expect("Empty access_token"); | ||
| } else { | ||
| eprintln!("[git pull] | repo provider is not github or bitbucket"); | ||
| return None; | ||
| } | ||
| return Some(access_token); |
There was a problem hiding this comment.
The get_access_token function is correctly handling the provider-specific logic for acquiring an access token. However, the repeated .to_string().to_lowercase() calls are unnecessary and can be replaced with direct enum comparisons.
if review.provider() == ProviderEnum::Bitbucket {
// Bitbucket logic
} else if review.provider() == ProviderEnum::Github {
// GitHub logic
}
| publish_hunkmap(&hunkmap); | ||
| let hunkmap_async = hunkmap.clone(); | ||
| let review_async = review.clone(); | ||
| let repo_config_clone = repo_config.clone(); | ||
| process_coverage(&hunkmap_async, &review_async, &repo_config_clone).await; | ||
| let mut repo_config_clone = repo_config.clone(); | ||
| process_coverage(&hunkmap_async, &review_async, &mut repo_config_clone, access_token).await; | ||
| } | ||
|
|
||
| fn hunk_already_exists(review: &Review) -> bool { |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [70-83]
The send_hunkmap function is now correctly handling the case where hunkmap_opt is None. The to_owned() call on an Option is unnecessary and can be replaced with a direct clone() on the contained value if it exists.
if let Some(hunkmap) = hunkmap_opt {
println!("HunkMap = {:?}", &hunkmap);
store_hunkmap_to_db(&hunkmap, review);
publish_hunkmap(&hunkmap);
process_coverage(hunkmap, review, repo_config, access_token).await;
} else {
eprintln!("Empty hunkmap in send_hunkmap");
}Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [85-95]
The hunk_already_exists function is using an expect call after an is_none() check, which is not recommended. Replace with proper error handling.
if let Some(hunkmap) = hunk_opt {
publish_hunkmap(&hunkmap);
println!("Hunk already in db!");
true
} else {
eprintln!("No hunk from get_hunk_from_db");
false
}
| pub async fn list_prs_github(repo_owner: &str, repo_name: &str, access_token: &str, state: &str) -> Option<Vec<String>> { | ||
| let headers_opt = prepare_headers(access_token); | ||
| if headers_opt.is_none() { | ||
| eprintln!("[list_prs_github] Unable to prepare auth headers for repository: {}", repo_name); | ||
| return None; | ||
| } | ||
| let headers = headers_opt.expect("Headers should be present"); | ||
|
|
||
| let mut params = HashMap::new(); | ||
| params.insert("state".to_string(), state.to_string()); | ||
|
|
||
| let pr_list_opt = get_list_prs_github(&headers, ¶ms, repo_owner, repo_name).await; | ||
| pr_list_opt | ||
| } |
There was a problem hiding this comment.
The function list_prs_github correctly handles the case where headers cannot be prepared by returning None. However, the use of expect on line 17 after checking is_none is redundant and could be replaced with an unwrap or by directly using the Some variant from headers_opt.
- let headers = headers_opt.expect("Headers should be present");
+ let headers = headers_opt.unwrap();Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| pub async fn list_prs_github(repo_owner: &str, repo_name: &str, access_token: &str, state: &str) -> Option<Vec<String>> { | |
| let headers_opt = prepare_headers(access_token); | |
| if headers_opt.is_none() { | |
| eprintln!("[list_prs_github] Unable to prepare auth headers for repository: {}", repo_name); | |
| return None; | |
| } | |
| let headers = headers_opt.expect("Headers should be present"); | |
| let mut params = HashMap::new(); | |
| params.insert("state".to_string(), state.to_string()); | |
| let pr_list_opt = get_list_prs_github(&headers, ¶ms, repo_owner, repo_name).await; | |
| pr_list_opt | |
| } | |
| pub async fn list_prs_github(repo_owner: &str, repo_name: &str, access_token: &str, state: &str) -> Option<Vec<String>> { | |
| let headers_opt = prepare_headers(access_token); | |
| if headers_opt.is_none() { | |
| eprintln!("[list_prs_github] Unable to prepare auth headers for repository: {}", repo_name); | |
| return None; | |
| } | |
| let headers = headers_opt.unwrap(); | |
| let mut params = HashMap::new(); | |
| params.insert("state".to_string(), state.to_string()); | |
| let pr_list_opt = get_list_prs_github(&headers, ¶ms, repo_owner, repo_name).await; | |
| pr_list_opt | |
| } |
| pub async fn list_prs_bitbucket(repo_owner: &str,repo_name: &str,access_token: &str,state: &str,) -> Option<Vec<String>> { | ||
| let headers_opt = prepare_auth_headers(access_token); | ||
| if headers_opt.is_none() { | ||
| eprintln!("[list_prs_bitbucket] Unable to prepare auth headers: {}", repo_name); | ||
| return None; | ||
| } | ||
| let headers = headers_opt.expect("Empty headers_opt"); | ||
| let headers = headers_opt.expect("Empty headers_opt"); | ||
| let mut params = HashMap::new(); | ||
| params.insert("state".to_string(), state.to_string()); | ||
| let pr_list_opt = get_list_prs(&headers, ¶ms, repo_owner, repo_name).await; |
There was a problem hiding this comment.
The function list_prs_bitbucket has been updated to include a new parameter state and uses a new helper function get_list_prs. The error handling has been improved by checking the result of prepare_auth_headers before proceeding. However, the use of expect after checking is_none is redundant and could be replaced with unwrap_or_else or similar error handling.
- let headers = headers_opt.expect("Empty headers_opt");
+ let headers = headers_opt.unwrap_or_else(|| {
+ eprintln!("[list_prs_bitbucket] Unable to prepare auth headers: {}", repo_name);
+ return None;
+ });Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| pub async fn list_prs_bitbucket(repo_owner: &str,repo_name: &str,access_token: &str,state: &str,) -> Option<Vec<String>> { | |
| let headers_opt = prepare_auth_headers(access_token); | |
| if headers_opt.is_none() { | |
| eprintln!("[list_prs_bitbucket] Unable to prepare auth headers: {}", repo_name); | |
| return None; | |
| } | |
| let headers = headers_opt.expect("Empty headers_opt"); | |
| let headers = headers_opt.expect("Empty headers_opt"); | |
| let mut params = HashMap::new(); | |
| params.insert("state".to_string(), state.to_string()); | |
| let pr_list_opt = get_list_prs(&headers, ¶ms, repo_owner, repo_name).await; | |
| pub async fn list_prs_bitbucket(repo_owner: &str,repo_name: &str,access_token: &str,state: &str,) -> Option<Vec<String>> { | |
| let headers_opt = prepare_auth_headers(access_token); | |
| if headers_opt.is_none() { | |
| eprintln!("[list_prs_bitbucket] Unable to prepare auth headers: {}", repo_name); | |
| return None; | |
| } | |
| let headers = headers_opt.unwrap_or_else(|| { | |
| eprintln!("[list_prs_bitbucket] Unable to prepare auth headers: {}", repo_name); | |
| return None; | |
| }); | |
| let mut params = HashMap::new(); | |
| params.insert("state".to_string(), state.to_string()); | |
| let pr_list_opt = get_list_prs(&headers, ¶ms, repo_owner, repo_name).await; |
| async fn process_message(attributes: &HashMap<String, String>, data_bytes: &Vec<u8>) { | ||
| let msgtype_opt = attributes.get("msgtype"); | ||
| if msgtype_opt.is_none() { |
There was a problem hiding this comment.
The early return pattern should be used to reduce nesting and improve readability. Also, the use of expect after checking is_none is redundant and can be replaced with unwrap_or_else or if let pattern.
if let Some(msgtype) = attributes.get("msgtype") {
// existing match logic
} else {
eprintln!("msgtype attribute not found in message : {:?}", attributes);
}
| let msgtype = msgtype_opt.expect("Empty msgtype"); | ||
| match msgtype.as_str() { | ||
| "install_callback" => { | ||
| println!("Processing install callback message"); | ||
| let msg_data_res = serde_json::from_slice::<InstallCallback>(data_bytes); | ||
| if msg_data_res.is_err() { | ||
| eprintln!("Error deserializing install callback: {:?}", msg_data_res); | ||
| return; | ||
| } | ||
| let data = msg_data_res.expect("msg_data not found"); | ||
| let code_async = data.installation_code.clone(); | ||
| task::spawn(async move { | ||
| handle_install_bitbucket(&code_async).await; | ||
| println!("Processed install callback message"); | ||
| }); | ||
| }, | ||
| process_install_callback(&data_bytes).await; | ||
| } | ||
| "webhook_callback" => { | ||
| let data_bytes_async = data_bytes.to_owned(); | ||
| let deserialized_data_opt = deserialized_data(&data_bytes_async); | ||
| let deserialised_msg_data = deserialized_data_opt.expect("Failed to deserialize data"); | ||
|
|
||
| let repo_provider = deserialised_msg_data["repositoryProvider"].to_string().trim_matches('"').to_string(); | ||
| let workspace_slug = deserialised_msg_data["eventPayload"]["repository"]["workspace"]["slug"].to_string().trim_matches('"').to_string(); | ||
| let repo_slug = deserialised_msg_data["eventPayload"]["repository"]["name"].to_string().trim_matches('"').to_string(); | ||
| let pr_number = deserialised_msg_data["eventPayload"]["pullrequest"]["id"].to_string().trim_matches('"').to_string(); | ||
| let event_type = deserialised_msg_data["eventType"].to_string().trim_matches('"').to_string(); | ||
| let is_reviewable = process_and_update_pr_if_different(&deserialised_msg_data["eventPayload"], &workspace_slug, &repo_slug, &pr_number, &repo_provider).await; | ||
|
|
||
| if event_type == "pullrequest:approved" { | ||
| todo!("Process approved event"); | ||
| } | ||
| if is_reviewable && (event_type == "pullrequest:created" || event_type == "pullrequest:updated" ) { | ||
| println!("[webhook_callback | deserialised_msg data] {} ", deserialised_msg_data); | ||
| let is_reviewable = process_and_update_pr_if_different(&deserialised_msg_data).await; | ||
| if is_reviewable { | ||
| task::spawn(async move { | ||
| process_review(&data_bytes_async).await; | ||
| println!("Processed webhook callback message"); | ||
| }); | ||
| } | ||
| } | ||
| _=> { | ||
| _ => { | ||
| eprintln!("Message type not found for message : {:?}", attributes); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The expect calls after is_none checks can lead to panics. It's better to handle errors gracefully. Use if let or match to handle the Option without expect.
if let Some(msgtype) = msgtype_opt {
// existing match logic
} else {
eprintln!("msgtype attribute not found in message : {:?}", attributes);
}
| async fn process_install_callback(data_bytes: &[u8]) { | ||
| println!("Processing install callback message"); | ||
| let msg_data_res = serde_json::from_slice::<InstallCallback>(data_bytes); | ||
| if msg_data_res.is_err() { | ||
| eprintln!("Error deserializing install callback: {:?}", msg_data_res); | ||
| return; | ||
| } | ||
| let data = msg_data_res.expect("msg_data not found"); | ||
| if data.repository_provider == ProviderEnum::Github.to_string().to_lowercase() { | ||
| let code_async = data.installation_code.clone(); | ||
| task::spawn(async move { | ||
| handle_install_github(&code_async).await; | ||
| println!("Processed install callback message"); | ||
| }); | ||
| } | ||
| if data.repository_provider == ProviderEnum::Bitbucket.to_string().to_lowercase() { | ||
| let code_async = data.installation_code.clone(); | ||
| task::spawn(async move { | ||
| handle_install_bitbucket(&code_async).await; | ||
| println!("Processed install callback message"); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The use of expect after checking is_err can lead to panics. Instead, handle the error with proper error logging and return early.
if let Ok(data) = msg_data_res {
// existing logic
} else {
eprintln!("Error deserializing install callback: {:?}", msg_data_res.err());
return;
}
| pub async fn get_pubsub_client_config(keypath: &str) -> ClientConfig { | ||
| let credfile = CredentialsFile::new_from_file(keypath.to_string()).await | ||
| let credfile = CredentialsFile::new_from_file(keypath.to_string()) | ||
| .await | ||
| .expect("Failed to locate credentials file"); | ||
| return ClientConfig::default() | ||
| .with_credentials(credfile) | ||
| .await | ||
| .expect("Unable to get PubSub Client config"); |
There was a problem hiding this comment.
The expect calls can lead to panics. It's better to handle errors gracefully and return an Result from the function to propagate the error.
pub async fn get_pubsub_client_config(keypath: &str) -> Result<ClientConfig, Box<dyn std::error::Error>> {
let credfile = CredentialsFile::new_from_file(keypath.to_string()).await?;
Ok(ClientConfig::default().with_credentials(credfile).await?)
}
| async fn setup_subscription(keypath: &str, topicname: &str) -> Subscription { | ||
| let config = get_pubsub_client_config(keypath).await; | ||
| let client = Client::new(config).await | ||
| let client = Client::new(config) | ||
| .await | ||
| .expect("Unable to create pubsub client to listen to messages"); | ||
| let topic = client.topic(topicname); | ||
| let topic_res = topic.exists(None).await; | ||
| if topic_res.is_err() { | ||
| let e = topic_res.expect_err("No error found in topic_res"); | ||
| if e.code() == Code::NotFound { | ||
| client.create_topic(topicname, None, None).await | ||
| client | ||
| .create_topic(topicname, None, None) | ||
| .await | ||
| .expect("Unable to create topic"); | ||
| } | ||
| else { | ||
| } else { | ||
| eprintln!("Error getting topic: {:?}", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
The expect calls can lead to panics. Instead, handle the error with proper error logging and return early. Also, consider returning a Result from the function to propagate the error.
if let Err(e) = topic_res {
if e.code() == Code::NotFound {
client.create_topic(topicname, None, None).await?;
} else {
eprintln!("Error getting topic: {:?}", e);
return Err(e.into());
}
}
| }; | ||
| let subscription_name = format!("{topicname}-sub"); | ||
| let subscription = client.subscription(&subscription_name); | ||
| let subconfig = SubscriptionConfig { | ||
| enable_message_ordering: true, | ||
| ..Default::default() | ||
| }; | ||
| if !subscription.exists(None).await.expect("Unable to get subscription information") { | ||
| subscription.create( | ||
| topic.fully_qualified_name(), subconfig, None) | ||
| .await.expect("Unable to create subscription for listening to messages"); | ||
| if !subscription | ||
| .exists(None) | ||
| .await | ||
| .expect("Unable to get subscription information") | ||
| { | ||
| subscription | ||
| .create(topic.fully_qualified_name(), sub_config, None) | ||
| .await | ||
| .expect("Unable to create subscription for listening to messages"); | ||
| } | ||
| println!("sub = {:?}", &subscription); | ||
| subscription |
There was a problem hiding this comment.
The expect calls can lead to panics. Instead, handle the error with proper error logging and return early. Also, consider returning a Result from the function to propagate the error.
if !subscription.exists(None).await? {
subscription.create(topic.fully_qualified_name(), sub_config, None).await?;
}
println!("sub = {:?}", &subscription);
Ok(subscription)
| let queue_cap = 100; | ||
| let mut message_hashes = VecDeque::with_capacity(queue_cap); | ||
| let subscription = setup_subscription(keypath, topicname).await; | ||
| let mut stream = subscription.subscribe(None).await | ||
| let mut stream = subscription | ||
| .subscribe(None) | ||
| .await | ||
| .expect("Unable to subscribe to messages"); | ||
| while let Some(message) = stream.next().await { | ||
| println!("Listening for messages..."); | ||
| let attrmap: HashMap<String, String> = message.message.attributes.clone().into_iter().collect(); | ||
| let attrmap: HashMap<String, String> = | ||
| message.message.attributes.clone().into_iter().collect(); | ||
| let message_hash = digest(&*message.message.data); | ||
| if !message_hashes.contains(&message_hash) { | ||
| message_hashes.push_back(message_hash); |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [130-146]
The expect call can lead to panics. Instead, handle the error with proper error logging and return early. Also, consider using a HashSet for message_hashes to improve performance on the contains check.
if let Err(e) = message.ack().await {
eprintln!("Failed to acknowledge message: {:?}", e);
}
| pub fn deserialized_data(message_data: &Vec<u8>) -> Option<Value> { | ||
| let msg_data_res = serde_json::from_slice::<Value>(message_data); | ||
| if msg_data_res.is_err() { | ||
| let e = msg_data_res.expect_err("No error in data_res"); | ||
| eprintln!("Incoming message does not contain valid reviews: {:?}", e); | ||
| return None; | ||
| } | ||
| let deserialized_data = msg_data_res.expect("Uncaught error in deserializing message_data"); | ||
| println!("deserialized_data == {:?}", &deserialized_data["eventPayload"]["repository"]); | ||
| let e = msg_data_res.expect_err("No error in data_res"); | ||
| eprintln!("Incoming message does not contain valid reviews: {:?}", e); | ||
| return None; | ||
| } | ||
| let deserialized_data = msg_data_res.expect("Uncaught error in deserializing message_data"); | ||
| println!( | ||
| "deserialized_data == {:?}", | ||
| &deserialized_data["eventPayload"]["repository"] | ||
| ); | ||
| Some(deserialized_data) | ||
| } |
There was a problem hiding this comment.
The expect calls can lead to panics. Instead, handle the error with proper error logging and return None.
match serde_json::from_slice::<Value>(message_data) {
Ok(data) => Some(data),
Err(e) => {
eprintln!("Incoming message does not contain valid reviews: {:?}", e);
None
}
}
| async fn process_and_update_pr_if_different(deserialised_msg_data: &Value) -> bool { | ||
| println!("[process_webhook_callback] {}", deserialised_msg_data); | ||
| let repo_provider = deserialised_msg_data["repositoryProvider"].to_string().trim_matches('"').to_string(); | ||
| let mut is_reviewable = false; | ||
| if repo_provider == ProviderEnum::Github.to_string().to_lowercase() { | ||
| let repo_owner = deserialised_msg_data["eventPayload"]["repository"]["owner"]["login"].to_string().trim_matches('"').to_string(); | ||
| let repo_name = deserialised_msg_data["eventPayload"]["repository"]["name"].to_string().trim_matches('"').to_string(); | ||
| let pr_number = deserialised_msg_data["eventPayload"]["pull_request"]["number"].to_string().trim_matches('"').to_string(); | ||
| let event_type = deserialised_msg_data["eventType"].to_string().trim_matches('"').to_string(); | ||
|
|
||
| println!("[process_webhook_callback] {}, {}, {}, {}", event_type, repo_owner, repo_name, pr_number); | ||
| if event_type == "pull_request_review" { | ||
| println!("[process_webhook_callback] Github PR review event"); | ||
| is_reviewable = github_process_and_update_pr_if_different(&deserialised_msg_data["eventPayload"], &repo_owner, &repo_name, &pr_number, &repo_provider).await; | ||
| } | ||
| if event_type == "pull_request" { | ||
| println!("[process_webhook_callback] Github PR opened"); | ||
| is_reviewable = github_process_and_update_pr_if_different(&deserialised_msg_data["eventPayload"], &repo_owner, &repo_name, &pr_number, &repo_provider).await; | ||
| } | ||
| } | ||
| if repo_provider == ProviderEnum::Bitbucket.to_string().to_lowercase() { | ||
| let workspace_slug = deserialised_msg_data["eventPayload"]["repository"]["workspace"]["slug"].to_string().trim_matches('"').to_string(); | ||
| let repo_slug = deserialised_msg_data["eventPayload"]["repository"]["name"].to_string().trim_matches('"').to_string(); | ||
| let pr_number = deserialised_msg_data["eventPayload"]["pullrequest"]["id"].to_string().trim_matches('"').to_string(); | ||
| let event_type = deserialised_msg_data["eventType"].to_string().trim_matches('"').to_string(); | ||
| let if_process_pr = bitbucket_process_and_update_pr_if_different(&deserialised_msg_data["eventPayload"],&workspace_slug,&repo_slug,&pr_number,&repo_provider, | ||
| ) | ||
| .await; | ||
|
|
||
| if event_type == "pullrequest:approved" { | ||
| is_reviewable = false; | ||
| todo!("Process approved event"); | ||
| }; | ||
| if if_process_pr && (event_type == "pullrequest:created" || event_type == "pullrequest:updated") { | ||
| is_reviewable = true; | ||
| }; | ||
| }; | ||
| return is_reviewable; | ||
|
|
There was a problem hiding this comment.
The expect calls can lead to panics. Instead, handle the error with proper error logging and return early. Also, consider using a match statement to handle the different event_type values more cleanly.
match event_type.as_str() {
"pull_request_review" => {
// ...
},
"pull_request" => {
// ...
},
// ...
}
| let client_id = std::env::var("BITBUCKET_CLIENT_ID") | ||
| .expect("BITBUCKET_CLIENT_ID must be set"); |
There was a problem hiding this comment.
The same issue with unwrap on environment variables as mentioned earlier. Use expect to provide a clear error message.
| return Some(refresh_token_resbody); | ||
| } | ||
|
|
||
| fn set_git_remote_url(git_url: &str, directory: &str, access_token: &str) { | ||
| let clone_url = git_url.to_string() | ||
| .replace("git@", format!("https://x-token-auth:{{{access_token}}}@").as_str()) | ||
| .replace("bitbucket.org:", "bitbucket.org/"); | ||
| let output = Command::new("git") | ||
| .arg("remote").arg("set-url").arg("origin") | ||
| .arg(clone_url) | ||
| .current_dir(directory) | ||
| .output() | ||
| .expect("failed to execute git pull"); | ||
| // Only for debug purposes | ||
| match str::from_utf8(&output.stderr) { | ||
| Ok(v) => println!("set_git_url stderr = {:?}", v), | ||
| Err(e) => eprintln!("set_git_url stderr error: {}", e), | ||
| }; | ||
| match str::from_utf8(&output.stdout) { | ||
| Ok(v) => println!("set_git_urll stdout = {:?}", v), | ||
| Err(e) => eprintln!("set_git_url stdout error: {}", e), | ||
| }; | ||
| println!("git pull output = {:?}, {:?}", &output.stdout, &output.stderr); | ||
| } | ||
|
|
||
| pub async fn get_access_token_review(review: &Review) -> Option<String> { | ||
| let authinfo_opt = auth_info(); | ||
| let authinfo_opt = bitbucket_auth_info(); | ||
| if authinfo_opt.is_none() { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
The repeated pattern of checking is_none and then unwrapping with expect is error-prone and can be replaced with if let or match constructs.
| for (blame_author, coverage) in coverage_floatmap.iter_mut() { | ||
| *coverage = *coverage / total * 100.0; | ||
| let formatted_value = format!("{:.2}", *coverage); | ||
| let user = get_workspace_user_from_db(blame_author); | ||
| if user.is_none() { | ||
| eprintln!("No user name found for {}", blame_author); | ||
| coverage_map.insert(blame_author.to_string(), formatted_value); | ||
| continue; | ||
| let coverage_key: String; | ||
| if repo_provider.to_string() == ProviderEnum::Bitbucket.to_string() { | ||
| let user = get_workspace_user_from_db(blame_author); | ||
| if user.is_none() { | ||
| eprintln!("No user name found for {}", blame_author); | ||
| coverage_map.insert(blame_author.to_string(), formatted_value); | ||
| continue; | ||
| } | ||
| let user_val = user.expect("user is empty"); | ||
| coverage_key = user_val.display_name().to_owned(); | ||
| } | ||
| else { | ||
| coverage_key = blame_author.to_string(); // TODO - get github user id and username here | ||
| } | ||
| let user_val = user.expect("user is empty"); | ||
| let coverage_key = user_val.display_name().to_owned(); | ||
| coverage_map.insert(coverage_key, formatted_value); |
There was a problem hiding this comment.
The logic for calculating coverage percentage and formatting it is correct. However, the database call to get_workspace_user_from_db for Bitbucket users should be handled more gracefully. Instead of using expect, which can panic, use a match statement or if-let construct to handle the None case.
- let user_val = user.expect("user is empty");
+ let user_val = match user {
+ Some(u) => u,
+ None => {
+ eprintln!("No user found for {}", blame_author);
+ continue;
+ }
+ };95:
The TODO comment indicates that GitHub user ID and username retrieval is not yet implemented. This needs to be addressed to ensure that GitHub users are correctly identified in the coverage map.
Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| for (blame_author, coverage) in coverage_floatmap.iter_mut() { | |
| *coverage = *coverage / total * 100.0; | |
| let formatted_value = format!("{:.2}", *coverage); | |
| let user = get_workspace_user_from_db(blame_author); | |
| if user.is_none() { | |
| eprintln!("No user name found for {}", blame_author); | |
| coverage_map.insert(blame_author.to_string(), formatted_value); | |
| continue; | |
| let coverage_key: String; | |
| if repo_provider.to_string() == ProviderEnum::Bitbucket.to_string() { | |
| let user = get_workspace_user_from_db(blame_author); | |
| if user.is_none() { | |
| eprintln!("No user name found for {}", blame_author); | |
| coverage_map.insert(blame_author.to_string(), formatted_value); | |
| continue; | |
| } | |
| let user_val = user.expect("user is empty"); | |
| coverage_key = user_val.display_name().to_owned(); | |
| } | |
| else { | |
| coverage_key = blame_author.to_string(); // TODO - get github user id and username here | |
| } | |
| let user_val = user.expect("user is empty"); | |
| let coverage_key = user_val.display_name().to_owned(); | |
| coverage_map.insert(coverage_key, formatted_value); | |
| for (blame_author, coverage) in coverage_floatmap.iter_mut() { | |
| *coverage = *coverage / total * 100.0; | |
| let formatted_value = format!("{:.2}", *coverage); | |
| let coverage_key: String; | |
| if repo_provider.to_string() == ProviderEnum::Bitbucket.to_string() { | |
| let user = get_workspace_user_from_db(blame_author); | |
| let user_val = match user { | |
| Some(u) => u, | |
| None => { | |
| eprintln!("No user found for {}", blame_author); | |
| continue; | |
| } | |
| }; | |
| coverage_key = user_val.display_name().to_owned(); | |
| } | |
| else { | |
| coverage_key = blame_author.to_string(); // TODO - get github user id and username here | |
| } | |
| coverage_map.insert(coverage_key, formatted_value); |
| if response_result.is_err() { | ||
| let e = response_result.expect_err("No error in sending request"); | ||
| eprintln!("[get_list_prs_github] Failed to send the request: {:?}", e); | ||
| return None; |
There was a problem hiding this comment.
Instead of calling expect_err after checking is_err, it would be more idiomatic to use if let Err(e) = response_result to handle the error.
- if response_result.is_err() {
- let e = response_result.expect_err("No error in sending request");
+ if let Err(e) = response_result {
eprintln!("[get_list_prs_github] Failed to send the request: {:?}", e);
return None;
}Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| if response_result.is_err() { | |
| let e = response_result.expect_err("No error in sending request"); | |
| eprintln!("[get_list_prs_github] Failed to send the request: {:?}", e); | |
| return None; | |
| if let Err(e) = response_result { | |
| eprintln!("[get_list_prs_github] Failed to send the request: {:?}", e); | |
| return None; | |
| } |
| let client = get_client(); | ||
| let base_url = bitbucket_base_url(); | ||
| let response_result = client | ||
| .get(&format!("{}/repositories/{}/{}/pullrequests", &base_url, repo_owner, repo_name)) | ||
| .get(&format!( | ||
| "{}/repositories/{}/{}/pullrequests", | ||
| &base_url, repo_owner, repo_name | ||
| )) | ||
| .headers(headers.to_owned()) | ||
| .query(params) | ||
| .send() |
There was a problem hiding this comment.
The get_list_prs function is correctly using the reqwest client to send an HTTP GET request. The headers and query parameters are being set appropriately. However, the error handling could be improved by avoiding the use of expect_err which will panic if there is no error. Instead, use if let or match to handle the error case.
- let e = response_result.expect_err("No error in sending request");
- eprintln!("[get_list_prs] Failed to send the request {:?}", e);
+ if let Err(e) = response_result {
+ eprintln!("[get_list_prs] Failed to send the request {:?}", e);
+ return None;
+ }Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| let client = get_client(); | |
| let base_url = bitbucket_base_url(); | |
| let response_result = client | |
| .get(&format!("{}/repositories/{}/{}/pullrequests", &base_url, repo_owner, repo_name)) | |
| .get(&format!( | |
| "{}/repositories/{}/{}/pullrequests", | |
| &base_url, repo_owner, repo_name | |
| )) | |
| .headers(headers.to_owned()) | |
| .query(params) | |
| .send() | |
| let client = get_client(); | |
| let base_url = bitbucket_base_url(); | |
| let response_result = client | |
| .get(&format!( | |
| "{}/repositories/{}/{}/pullrequests", | |
| &base_url, repo_owner, repo_name | |
| )) | |
| .headers(headers.to_owned()) | |
| .query(params) | |
| .send(); | |
| if let Err(e) = response_result { | |
| eprintln!("[get_list_prs] Failed to send the request {:?}", e); | |
| return None; | |
| } |
| let parse_result = response.json::<Value>().await; | ||
| if parse_result.is_err() { | ||
| let e = parse_result.expect_err("No error in parsing"); | ||
| eprintln!( | ||
| "[get_list_prs_github] Failed to parse JSON: {:?}", | ||
| e | ||
| ); | ||
| return None; | ||
| } |
There was a problem hiding this comment.
Similar to the previous comment, use if let Err(e) = parse_result instead of expect_err after is_err.
- if parse_result.is_err() {
- let e = parse_result.expect_err("No error in parsing");
+ if let Err(e) = parse_result {
eprintln!(
"[get_list_prs_github] Failed to parse JSON: {:?}",
e
);
return None;
}Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| let parse_result = response.json::<Value>().await; | |
| if parse_result.is_err() { | |
| let e = parse_result.expect_err("No error in parsing"); | |
| eprintln!( | |
| "[get_list_prs_github] Failed to parse JSON: {:?}", | |
| e | |
| ); | |
| return None; | |
| } | |
| let parse_result = response.json::<Value>().await; | |
| if let Err(e) = parse_result { | |
| eprintln!( | |
| "[get_list_prs_github] Failed to parse JSON: {:?}", | |
| e | |
| ); | |
| return None; | |
| } |
|
|
||
| let response = response_result.expect("Uncaught error in parsing response"); | ||
| if !response.status().is_success() { | ||
| eprintln!("[get_list_prs] Request failed with status: {:?}", response.status()); | ||
| eprintln!( | ||
| "[get_list_prs] Request failed with status: {:?}", | ||
| response.status() | ||
| ); | ||
| return None; | ||
| } | ||
|
|
||
| let parse_result = response.json::<Value>().await; | ||
| if parse_result.is_err() { | ||
| let parse_result_err = parse_result.expect_err("No error in parsing"); | ||
| eprintln!("[get_list_prs] Failed to parse JSON: {:?}", parse_result_err); | ||
| eprintln!( | ||
| "[get_list_prs] Failed to parse JSON: {:?}", | ||
| parse_result_err | ||
| ); | ||
| return None; | ||
| } |
There was a problem hiding this comment.
The error handling in this section could be improved by using if let or match instead of expect_err and expect which can cause panics. The code should handle errors gracefully without panicking.
- let parse_result_err = parse_result.expect_err("No error in parsing");
- eprintln!("[get_list_prs] Failed to parse JSON: {:?}", parse_result_err);
+ if let Err(parse_result_err) = parse_result {
+ eprintln!("[get_list_prs] Failed to parse JSON: {:?}", parse_result_err);
+ return None;
+ }Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| let response = response_result.expect("Uncaught error in parsing response"); | |
| if !response.status().is_success() { | |
| eprintln!("[get_list_prs] Request failed with status: {:?}", response.status()); | |
| eprintln!( | |
| "[get_list_prs] Request failed with status: {:?}", | |
| response.status() | |
| ); | |
| return None; | |
| } | |
| let parse_result = response.json::<Value>().await; | |
| if parse_result.is_err() { | |
| let parse_result_err = parse_result.expect_err("No error in parsing"); | |
| eprintln!("[get_list_prs] Failed to parse JSON: {:?}", parse_result_err); | |
| eprintln!( | |
| "[get_list_prs] Failed to parse JSON: {:?}", | |
| parse_result_err | |
| ); | |
| return None; | |
| } | |
| let response = response_result.expect("Uncaught error in parsing response"); | |
| if !response.status().is_success() { | |
| eprintln!( | |
| "[get_list_prs] Request failed with status: {:?}", | |
| response.status() | |
| ); | |
| return None; | |
| } | |
| let parse_result = response.json::<Value>().await; | |
| if let Err(parse_result_err) = parse_result { | |
| eprintln!( | |
| "[get_list_prs] Failed to parse JSON: {:?}", | |
| parse_result_err | |
| ); | |
| return None; | |
| } |
| let pr_list = prs_data.as_array() | ||
| .expect("Expected an array of PRs") | ||
| .iter() | ||
| .map(|pr| pr["number"].to_string()) | ||
| .collect(); | ||
|
|
||
| Some(pr_list) |
There was a problem hiding this comment.
The code assumes that the JSON response will be an array of PRs. While this is likely correct, it would be safer to handle the case where the response is not an array or the "number" field is missing. This could be done by using filter_map and checking if pr["number"] is a valid number.
let pr_list = prs_data.as_array()
.unwrap_or(&Vec::new())
.iter()
.filter_map(|pr| pr["number"].as_i64())
.map(|num| num.to_string())
.collect();
| if response_result.is_err() { | ||
| let e = response_result.expect_err("No error in getting PR response"); | ||
| eprintln!("Error getting PR info: {:?}", e); | ||
| return None; | ||
| } |
There was a problem hiding this comment.
Again, use if let Err(e) = response_result instead of expect_err after is_err.
- if response_result.is_err() {
- let e = response_result.expect_err("No error in getting PR response");
+ if let Err(e) = response_result {
eprintln!("Error getting PR info: {:?}", e);
return None;
}Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| if response_result.is_err() { | |
| let e = response_result.expect_err("No error in getting PR response"); | |
| eprintln!("Error getting PR info: {:?}", e); | |
| return None; | |
| } | |
| if let Err(e) = response_result { | |
| eprintln!("Error getting PR info: {:?}", e); | |
| return None; | |
| } |
| let parse_result = response.json::<Value>().await; | ||
| if parse_result.is_err() { | ||
| let e = parse_result.expect_err("No error in parsing"); | ||
| eprintln!("Error parsing PR data: {:?}", e); | ||
| return None; | ||
| } |
There was a problem hiding this comment.
Use if let Err(e) = parse_result instead of expect_err after is_err.
- if parse_result.is_err() {
- let e = parse_result.expect_err("No error in parsing");
+ if let Err(e) = parse_result {
eprintln!("Error parsing PR data: {:?}", e);
return None;
}Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| let parse_result = response.json::<Value>().await; | |
| if parse_result.is_err() { | |
| let e = parse_result.expect_err("No error in parsing"); | |
| eprintln!("Error parsing PR data: {:?}", e); | |
| return None; | |
| } | |
| let parse_result = response.json::<Value>().await; | |
| if let Err(e) = parse_result { | |
| eprintln!("Error parsing PR data: {:?}", e); | |
| return None; | |
| } |
| eprintln!("[get_list_prs] Unable to parse get_list_prs: {:?}", e); | ||
| return None; | ||
| } | ||
| let pr_list_parsed: Vec<Value> = pr_list_parse_res.expect("Uncaught error in pr_list_parse_res"); | ||
| let pr_list_parsed: Vec<Value> = | ||
| pr_list_parse_res.expect("Uncaught error in pr_list_parse_res"); | ||
| let mut pr_list: Vec<String> = Vec::new(); | ||
| for pr in pr_list_parsed.iter() { | ||
| pr_list.push(pr["id"].to_string().trim_matches('"').to_string()); | ||
| } | ||
| if pr_list.is_empty() { | ||
| eprintln!("[get_list_prs] pr_list is empty for parsed value: {:?}", &pr_list_parsed); | ||
| eprintln!( | ||
| "[get_list_prs] pr_list is empty for parsed value: {:?}", | ||
| &pr_list_parsed | ||
| ); | ||
| return None; | ||
| } | ||
| return Some(pr_list); |
There was a problem hiding this comment.
The error handling for pr_list_parse_res is also using expect_err which is not recommended. Use if let or match for error handling. Additionally, the code could be refactored to reduce nesting and improve readability.
- let e = pr_list_parse_res.expect_err("Empty error in pr_list_parse_res");
- eprintln!("[get_list_prs] Unable to parse get_list_prs: {:?}", e);
+ if let Err(e) = pr_list_parse_res {
+ eprintln!("[get_list_prs] Unable to parse get_list_prs: {:?}", e);
+ return None;
+ }Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| eprintln!("[get_list_prs] Unable to parse get_list_prs: {:?}", e); | |
| return None; | |
| } | |
| let pr_list_parsed: Vec<Value> = pr_list_parse_res.expect("Uncaught error in pr_list_parse_res"); | |
| let pr_list_parsed: Vec<Value> = | |
| pr_list_parse_res.expect("Uncaught error in pr_list_parse_res"); | |
| let mut pr_list: Vec<String> = Vec::new(); | |
| for pr in pr_list_parsed.iter() { | |
| pr_list.push(pr["id"].to_string().trim_matches('"').to_string()); | |
| } | |
| if pr_list.is_empty() { | |
| eprintln!("[get_list_prs] pr_list is empty for parsed value: {:?}", &pr_list_parsed); | |
| eprintln!( | |
| "[get_list_prs] pr_list is empty for parsed value: {:?}", | |
| &pr_list_parsed | |
| ); | |
| return None; | |
| } | |
| return Some(pr_list); | |
| if let Err(e) = pr_list_parse_res { | |
| eprintln!("[get_list_prs] Unable to parse get_list_prs: {:?}", e); | |
| return None; | |
| } | |
| let pr_list_parsed: Vec<Value> = | |
| pr_list_parse_res.expect("Uncaught error in pr_list_parse_res"); | |
| let mut pr_list: Vec<String> = Vec::new(); | |
| for pr in pr_list_parsed.iter() { | |
| pr_list.push(pr["id"].to_string().trim_matches('"').to_string()); | |
| } | |
| if pr_list.is_empty() { | |
| eprintln!( | |
| "[get_list_prs] pr_list is empty for parsed value: {:?}", | |
| &pr_list_parsed | |
| ); | |
| return None; | |
| } | |
| return Some(pr_list); |
| let pr_info = PrInfo { | ||
| base_head_commit: pr_data["base"]["sha"].as_str()?.to_string(), | ||
| pr_head_commit: pr_data["head"]["sha"].as_str()?.to_string(), | ||
| state: pr_data["state"].as_str()?.to_string(), | ||
| pr_branch: pr_data["head"]["ref"].as_str()?.to_string(), | ||
| }; |
There was a problem hiding this comment.
The code does not handle the case where the JSON fields are missing or not strings. It would be safer to handle these cases gracefully.
let pr_info = PrInfo {
base_head_commit: pr_data["base"]["sha"].as_str().unwrap_or_default().to_string(),
pr_head_commit: pr_data["head"]["sha"].as_str().unwrap_or_default().to_string(),
state: pr_data["state"].as_str().unwrap_or_default().to_string(),
pr_branch: pr_data["head"]["ref"].as_str().unwrap_or_default().to_string(),
};
| pub async fn get_and_store_pr_info(repo_owner: &str, repo_name: &str, access_token: &str, pr_number: &str) { | ||
| let repo_provider = "github"; | ||
| if let Some(pr_info) = get_pr_info_github(repo_owner, repo_name, access_token, pr_number).await { | ||
| // If PR information is available, store it in the database | ||
| update_pr_info_in_db(repo_owner, repo_name, &pr_info, pr_number, repo_provider).await; | ||
| } else { | ||
| eprintln!("No PR info available for PR number: {:?} repository: {:?} repo_owner{:?}", pr_number, repo_name, repo_owner); | ||
| } | ||
| } |
There was a problem hiding this comment.
The function get_and_store_pr_info correctly handles the case where PR information is not available. However, the error message on line 128 could be improved by using consistent formatting for the placeholders.
- eprintln!("No PR info available for PR number: {:?} repository: {:?} repo_owner{:?}", pr_number, repo_name, repo_owner);
+ eprintln!("No PR info available for PR number: {} in repository: {} owned by {}", pr_number, repo_name, repo_owner);Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| pub async fn get_and_store_pr_info(repo_owner: &str, repo_name: &str, access_token: &str, pr_number: &str) { | |
| let repo_provider = "github"; | |
| if let Some(pr_info) = get_pr_info_github(repo_owner, repo_name, access_token, pr_number).await { | |
| // If PR information is available, store it in the database | |
| update_pr_info_in_db(repo_owner, repo_name, &pr_info, pr_number, repo_provider).await; | |
| } else { | |
| eprintln!("No PR info available for PR number: {:?} repository: {:?} repo_owner{:?}", pr_number, repo_name, repo_owner); | |
| } | |
| } | |
| pub async fn get_and_store_pr_info(repo_owner: &str, repo_name: &str, access_token: &str, pr_number: &str) { | |
| let repo_provider = "github"; | |
| if let Some(pr_info) = get_pr_info_github(repo_owner, repo_name, access_token, pr_number).await { | |
| // If PR information is available, store it in the database | |
| update_pr_info_in_db(repo_owner, repo_name, &pr_info, pr_number, repo_provider).await; | |
| } else { | |
| eprintln!("No PR info available for PR number: {} in repository: {} owned by {}", pr_number, repo_name, repo_owner); | |
| } | |
| } |
| eprintln!("[get_list_prs] Unable to parse get_list_prs: {:?}", e); | ||
| return None; | ||
| } | ||
| let pr_list_parsed: Vec<Value> = pr_list_parse_res.expect("Uncaught error in pr_list_parse_res"); | ||
| let pr_list_parsed: Vec<Value> = | ||
| pr_list_parse_res.expect("Uncaught error in pr_list_parse_res"); | ||
| let mut pr_list: Vec<String> = Vec::new(); | ||
| for pr in pr_list_parsed.iter() { | ||
| pr_list.push(pr["id"].to_string().trim_matches('"').to_string()); | ||
| } | ||
| if pr_list.is_empty() { | ||
| eprintln!("[get_list_prs] pr_list is empty for parsed value: {:?}", &pr_list_parsed); | ||
| eprintln!( | ||
| "[get_list_prs] pr_list is empty for parsed value: {:?}", | ||
| &pr_list_parsed | ||
| ); | ||
| return None; | ||
| } | ||
| return Some(pr_list); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| pub async fn get_pr_info(workspace_slug: &str, repo_slug: &str, access_token: &str, pr_number: &str) -> Option<PrInfo> { | ||
| pub async fn get_pr_info(workspace_slug: &str,repo_slug: &str,access_token: &str,pr_number: &str) -> Option<PrInfo> { | ||
| let base_url = bitbucket_base_url(); | ||
| let url = format!("{}/repositories/{}/{}/pullrequests/{}", &base_url, workspace_slug, repo_slug, pr_number); | ||
| let url = format!( | ||
| "{}/repositories/{}/{}/pullrequests/{}", | ||
| &base_url, workspace_slug, repo_slug, pr_number | ||
| ); | ||
| println!("[get_pr_info] url: {:?}", &url); | ||
| println!("[get_pr_info] access token: {:?}", access_token); | ||
| let client = get_client(); | ||
| let response_result = client.get(&url) | ||
| let response_result = client | ||
| .get(&url) | ||
| .header("Authorization", format!("Bearer {}", access_token)) | ||
| .header("Accept", "application/json") | ||
| .send() | ||
| .await; | ||
|
|
||
| if response_result.is_err() { | ||
| let res_err = response_result.expect_err("No error in getting Pr response"); | ||
| println!("Error getting PR info: {:?}", res_err); |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [82-117]
The function get_pr_info has been updated to use the println! macro for logging, which is not ideal for production code as it does not allow for log levels or easy redirection to different logging outputs. Consider using a logging framework like log or env_logger. Also, the error handling could be improved by avoiding the use of expect which can panic.
- let pr_data: Value = response.json().await.expect("Error parsing PR data");
+ let pr_data_result = response.json::<Value>().await;
+ let pr_data = match pr_data_result {
+ Ok(data) => data,
+ Err(e) => {
+ println!("Error parsing PR data: {:?}", e);
+ return None;
+ }
+ };
Please check the action items covered in the PR -
Summary by CodeRabbit
New Features
Refactor
get_clientfunction to a shared utility module for reusability.Bug Fixes
Documentation
Style
Tests
Chores
Revert