Skip to content

Commit fd32535

Browse files
authored
Merge pull request #1906 from Kobzol/assigned-prs-in-memory
Store assigned PRs in memory
2 parents 60c2593 + c4a6179 commit fd32535

10 files changed

+194
-334
lines changed

src/github.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ use std::{
1515
};
1616
use tracing as log;
1717

18-
#[derive(Debug, PartialEq, Eq, serde::Deserialize, Clone)]
18+
pub type UserId = u64;
19+
20+
#[derive(Debug, PartialEq, Eq, Hash, serde::Deserialize, Clone)]
1921
pub struct User {
2022
pub login: String,
21-
pub id: u64,
23+
pub id: UserId,
2224
}
2325

2426
impl GithubClient {
@@ -3048,7 +3050,7 @@ async fn project_items_by_status(
30483050
}
30493051

30503052
/// Retrieve all pull requests in status OPEN that are not drafts
3051-
pub async fn retrieve_pull_requests(
3053+
pub async fn retrieve_open_pull_requests(
30523054
repo: &Repository,
30533055
client: &GithubClient,
30543056
) -> anyhow::Result<Vec<(User, i32)>> {

src/handlers.rs

+4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::config::{self, Config, ConfigurationError};
22
use crate::github::{Event, GithubClient, IssueCommentAction, IssuesAction, IssuesEvent};
3+
use crate::handlers::pr_tracking::ReviewerWorkqueue;
34
use octocrab::Octocrab;
45
use parser::command::{assign::AssignCommand, Command, Input};
56
use std::fmt;
@@ -363,4 +364,7 @@ pub struct Context {
363364
pub db: crate::db::ClientPool,
364365
pub username: String,
365366
pub octocrab: Octocrab,
367+
/// Represents the workqueue (assigned open PRs) of individual reviewers.
368+
/// tokio's RwLock is used to avoid deadlocks, since we run on a single-threaded tokio runtime.
369+
pub workqueue: Arc<tokio::sync::RwLock<ReviewerWorkqueue>>,
366370
}

src/handlers/assign.rs

+36-63
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
use crate::{
2424
config::{AssignConfig, WarnNonDefaultBranchException},
2525
github::{self, Event, FileDiff, Issue, IssuesAction, Selection},
26-
handlers::{pr_tracking::has_user_capacity, Context, GithubClient, IssuesEvent},
26+
handlers::{Context, GithubClient, IssuesEvent},
2727
interactions::EditIssueBody,
2828
};
2929
use anyhow::{bail, Context as _};
@@ -39,7 +39,6 @@ use tracing as log;
3939
#[cfg(test)]
4040
mod tests {
4141
mod tests_candidates;
42-
mod tests_db;
4342
mod tests_from_diff;
4443
}
4544

@@ -564,22 +563,22 @@ pub(super) async fn handle_command(
564563
}
565564
let db_client = ctx.db.get().await;
566565
if is_self_assign(&name, &event.user().login) {
567-
let work_queue = has_user_capacity(&db_client, &name).await;
568-
if work_queue.is_err() {
569-
// NOTE: disabled for now, just log
570-
log::warn!(
571-
"[#{}] PR self-assign failed, DB reported that user {} has no review capacity. Ignoring.",
572-
issue.number,
573-
name
574-
);
575-
// issue
576-
// .post_comment(
577-
// &ctx.github,
578-
// &REVIEWER_HAS_NO_CAPACITY.replace("{username}", &name),
579-
// )
580-
// .await?;
581-
// return Ok(());
582-
}
566+
// let work_queue = has_user_capacity(&db_client, &name).await;
567+
// if work_queue.is_err() {
568+
// // NOTE: disabled for now, just log
569+
// log::warn!(
570+
// "[#{}] PR self-assign failed, DB reported that user {} has no review capacity. Ignoring.",
571+
// issue.number,
572+
// name
573+
// );
574+
// // issue
575+
// // .post_comment(
576+
// // &ctx.github,
577+
// // &REVIEWER_HAS_NO_CAPACITY.replace("{username}", &name),
578+
// // )
579+
// // .await?;
580+
// // return Ok(());
581+
// }
583582

584583
name.to_string()
585584
} else {
@@ -806,7 +805,7 @@ impl fmt::Display for FindReviewerError {
806805
/// auto-assign groups, or rust-lang team names. It must have at least one
807806
/// entry.
808807
async fn find_reviewer_from_names(
809-
db: &DbClient,
808+
_db: &DbClient,
810809
teams: &Teams,
811810
config: &AssignConfig,
812811
issue: &Issue,
@@ -845,24 +844,24 @@ async fn find_reviewer_from_names(
845844
}
846845

847846
// filter out team members without capacity
848-
let filtered_candidates = filter_by_capacity(db, &candidates)
849-
.await
850-
.expect("Error while filtering out team members");
851-
852-
if filtered_candidates.is_empty() {
853-
// NOTE: disabled for now, just log
854-
log::info!("[#{}] Filtered list of PR assignee is empty", issue.number);
855-
// return Err(FindReviewerError::AllReviewersFiltered {
856-
// initial: names.to_vec(),
857-
// filtered: names.to_vec(),
858-
// });
859-
}
860-
861-
log::info!(
862-
"[#{}] Filtered list of candidates: {:?}",
863-
issue.number,
864-
filtered_candidates
865-
);
847+
// let filtered_candidates = filter_by_capacity(db, &candidates)
848+
// .await
849+
// .expect("Error while filtering out team members");
850+
//
851+
// if filtered_candidates.is_empty() {
852+
// // NOTE: disabled for now, just log
853+
// log::info!("[#{}] Filtered list of PR assignee is empty", issue.number);
854+
// // return Err(FindReviewerError::AllReviewersFiltered {
855+
// // initial: names.to_vec(),
856+
// // filtered: names.to_vec(),
857+
// // });
858+
// }
859+
//
860+
// log::info!(
861+
// "[#{}] Filtered list of candidates: {:?}",
862+
// issue.number,
863+
// filtered_candidates
864+
// );
866865

867866
// Return unfiltered list of candidates
868867
Ok(candidates
@@ -872,32 +871,6 @@ async fn find_reviewer_from_names(
872871
.to_string())
873872
}
874873

875-
/// Filter out candidates not having review capacity
876-
async fn filter_by_capacity(
877-
db: &DbClient,
878-
candidates: &HashSet<&str>,
879-
) -> anyhow::Result<HashSet<String>> {
880-
let usernames = candidates
881-
.iter()
882-
.map(|c| *c)
883-
.collect::<Vec<&str>>()
884-
.join(",");
885-
886-
let q = format!(
887-
"
888-
SELECT username
889-
FROM review_prefs r
890-
JOIN users on users.user_id=r.user_id
891-
AND username = ANY('{{ {} }}')
892-
AND CARDINALITY(r.assigned_prs) < LEAST(COALESCE(r.max_assigned_prs,1000000))",
893-
usernames
894-
);
895-
let result = db.query(&q, &[]).await.context("Select DB error")?;
896-
let candidates: HashSet<String> = result.iter().map(|row| row.get("username")).collect();
897-
log::info!("DB returned these candidates: {:?}", candidates);
898-
Ok(candidates)
899-
}
900-
901874
/// Returns a list of candidate usernames (from relevant teams) to choose as a reviewer.
902875
fn candidate_reviewers_from_names<'a>(
903876
teams: &'a Teams,

src/handlers/assign/tests/tests_db.rs

-34
This file was deleted.

0 commit comments

Comments
 (0)