Tr/ablejobs #46
Tr/ablejobs #46
Conversation
Deploying all bugfixes to ablejobs
…ate, and didn't run update-pr-in-db on creation
…low-bugfix Akg/webhook callback flow bugfix
WalkthroughThe changes introduced a new Docker build command with several build arguments and added new functionalities to the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 13
Configuration used: CodeRabbit UI
Files selected for processing (10)
- ablejobs_command.txt (1 hunks)
- vibi-dpu/src/bitbucket/mod.rs (1 hunks)
- vibi-dpu/src/bitbucket/prs.rs (1 hunks)
- vibi-dpu/src/core/review.rs (1 hunks)
- vibi-dpu/src/core/setup.rs (4 hunks)
- vibi-dpu/src/db/mod.rs (1 hunks)
- vibi-dpu/src/db/prs.rs (1 hunks)
- vibi-dpu/src/pubsub/listener.rs (4 hunks)
- vibi-dpu/src/utils/mod.rs (1 hunks)
- vibi-dpu/src/utils/pr_info.rs (1 hunks)
Files skipped from review due to trivial changes (5)
- vibi-dpu/src/bitbucket/mod.rs
- vibi-dpu/src/core/review.rs
- vibi-dpu/src/db/mod.rs
- vibi-dpu/src/utils/mod.rs
- vibi-dpu/src/utils/pr_info.rs
Additional comments: 9
vibi-dpu/src/pubsub/listener.rs (6)
1-4: The import statements are clean and organized. Good job.
7-18: The import statements are well organized and the
InstallCallbackstruct is correctly defined.77-100: The
prcoess_install_callbackfunction seems to have a typo in its name. It should beprocess_install_callback. Also, the function handles different repository providers correctly. However, the Github case is commented out and marked as "To be Implemented". Make sure to implement this case before merging.- async fn prcoess_install_callback(data_bytes: &[u8]) { + async fn process_install_callback(data_bytes: &[u8]) {
- 46-104: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [102-105]
The
get_pubsub_client_configfunction is correctly implemented and handles errors gracefully.
- 1-4: > Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [106-165]
The
setup_subscriptionandlisten_messagesfunctions are correctly implemented and handle errors gracefully.
- 171-181: The
deserialized_datafunction is correctly implemented and handles errors gracefully.vibi-dpu/src/core/setup.rs (3)
3-8: No changes were made in this hunk. The imports are standard and necessary for the rest of the code.
12-18: The import statements have been updated to include the new functions
list_prs_bitbucketandget_and_store_pr_infofrom theprsmodule. Ensure that these functions are correctly implemented and tested.94-100: No changes were made in this hunk. The
send_setup_infofunction is correctly implemented and handles errors appropriately.
2. removed duplicated function (save and update pr_info functions were doing the same thing)
bug fix ablejobs
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Configuration used: CodeRabbit UI
Files selected for processing (3)
- vibi-dpu/src/bitbucket/prs.rs (1 hunks)
- vibi-dpu/src/db/prs.rs (1 hunks)
- vibi-dpu/src/pubsub/listener.rs (4 hunks)
Additional comments: 5
vibi-dpu/src/bitbucket/prs.rs (3)
9-20: The function
list_prs_bitbuckethas been updated to returnOptioninstead ofResult. This change is acceptable as it simplifies the error handling process. However, it's important to ensure that all calls to this function throughout the codebase have been updated to handle the new return type.22-66: The function
get_list_prshas been updated to returnOptioninstead ofResult. This change is acceptable as it simplifies the error handling process. However, it's important to ensure that all calls to this function throughout the codebase have been updated to handle the new return type.103-110: The function
get_and_store_pr_inforetrieves and stores PR information. It's well written and handles errors gracefully. However, it's important to ensure that all calls to this function throughout the codebase have been updated to handle the new return type.vibi-dpu/src/pubsub/listener.rs (2)
1-4: The import statement for
std::collections::HashMaphas been removed but it is still being used in the code. Please verify if this is intentional.170-180: The
deserialized_datafunction has been added to deserialize message data into aserde_json::Valueobject. This function is well implemented and handles errors gracefully.
* bug fix: removed IVec of value while inserting pr_info * bug fix: database key changed for PR-info to resolve key collision.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files selected for processing (2)
- vibi-dpu/src/db/prs.rs (1 hunks)
- vibi-dpu/src/pubsub/listener.rs (4 hunks)
Additional comments: 4
vibi-dpu/src/pubsub/listener.rs (3)
1-4: The import statements are well organized and only necessary modules are imported. Good job on that.
7-16: The import statements are well organized and only necessary modules are imported. Good job on that.
7-18: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [18-22]
The
InstallCallbackstruct is well defined with the necessary fields.vibi-dpu/src/db/prs.rs (1)
- 55-72: The function
parse_webhook_datashould return aResultinstead of just printing the error and returning anOption. This would allow the caller to handle the error in a way that makes sense in its context.- fn parse_webhook_data(webhook_data: &Value) -> Option<PrInfo> { + fn parse_webhook_data(webhook_data: &Value) -> Result<PrInfo, Box<dyn std::error::Error>> {
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- vibi-dpu/src/bitbucket/prs.rs (3 hunks)
Additional comments: 2
vibi-dpu/src/bitbucket/prs.rs (2)
2-5: The import statement for the
save_pr_info_to_dbfunction has been replaced with an import statement for theupdate_pr_info_in_dbfunction. Ensure that theupdate_pr_info_in_dbfunction is implemented and behaves as expected.103-109: The
save_pr_info_to_dbfunction call has been replaced with anupdate_pr_info_in_dbfunction call. Ensure that theupdate_pr_info_in_dbfunction is implemented and behaves as expected.
| return None; | ||
| } | ||
| let pr_data: Value = response.json().await.expect("Error parsing PR data"); | ||
|
|
||
| Some(PrInfo { | ||
| let pr_info = PrInfo { | ||
| base_head_commit: pr_data["destination"]["commit"]["hash"].to_string().trim_matches('"').to_string(), | ||
| pr_head_commit: pr_data["source"]["commit"]["hash"].to_string().trim_matches('"').to_string(), | ||
| state: pr_data["state"].to_string().trim_matches('"').to_string(), | ||
| pr_branch: pr_data["source"]["branch"]["name"].to_string().trim_matches('"').to_string(), | ||
| }) | ||
| }; |
There was a problem hiding this comment.
The PrInfo struct is being populated directly from the JSON response without any error checking. If the response does not contain the expected fields, this could cause a panic. Consider adding error handling to ensure that the expected fields are present in the response.
let pr_info = match (pr_data["destination"]["commit"]["hash"].as_str(), pr_data["source"]["commit"]["hash"].as_str(), pr_data["state"].as_str(), pr_data["source"]["branch"]["name"].as_str()) {
(Some(base_head_commit), Some(pr_head_commit), Some(state), Some(pr_branch)) => PrInfo {
base_head_commit: base_head_commit.to_string(),
pr_head_commit: pr_head_commit.to_string(),
state: state.to_string(),
pr_branch: pr_branch.to_string(),
},
_ => {
eprintln!("Failed to parse PR data: {:?}", pr_data);
return None;
}
};
|
Kudos, SonarCloud Quality Gate passed!
|
Please check the action items covered in the PR -
Summary by CodeRabbit
New Features:
PrInfoto better manage pull request information.Refactor:
Chores:
reqwest,serde_json, andstd::collections::HashMap.