Store assigned PRs in memory #1906
Conversation
Kobzol
force-pushed
the
assigned-prs-in-memory
branch
from
1a93fe6 to
3fde3dd
Compare
Kobzol
force-pushed
the
assigned-prs-in-memory
branch
from
3fde3dd to
d64caa6
Compare
|
Rebased. |
| @@ -348,4 +349,7 @@ pub struct Context { | |||
| pub db: crate::db::ClientPool, | |||
| pub username: String, | |||
| pub octocrab: Octocrab, | |||
| /// Represents the workqueue (assigned open PRs) of individual reviewers. | |||
| /// tokio's RwLock is used to avoid deadlocks, since we run on a single-threaded tokio runtime. | |||
| pub workqueue: Arc<tokio::sync::RwLock<ReviewerWorkqueue>>, | |||
There was a problem hiding this comment.
So, I don't quite like storing this state like this - I'd rather have some (Handler, HandlerState) somewhere. But this is fine for now.
There was a problem hiding this comment.
The problem is that this state has to be available across handlers. It is populated in pull_requests_assignment_update.rs, updated in pr_tracking.rs and it will be read in the future in assign.rs. Thus it's pretty much global state, hence it being stored in Context.
| // .await?; | ||
| // return Ok(()); | ||
| } | ||
| // let work_queue = has_user_capacity(&db_client, &name).await; |
There was a problem hiding this comment.
Why is this disabled? We definitely want this logging.
There was a problem hiding this comment.
These functions that I commented were using the assigned_prs column in the DB, which is no longer used after this PR. So these functions simply do not work anymore (in fact I removed them in this PR).
I'm planning to further work on assigning PR capacity limit, and then refactor the way PRs are assigned, and add back logging (and then we can hopefully enable the new assignment logic again).
I did not want to do all that in this PR, hence I commented this code.
| issue.number, | ||
| filtered_candidates | ||
| ); | ||
| // let filtered_candidates = filter_by_capacity(db, &candidates) |
Kobzol
force-pushed
the
assigned-prs-in-memory
branch
from
d64caa6 to
b7c8cd8
Compare
|
Let me know if you want further changes :) |
|
@Kobzol I'm holding off merging, since I think we should make sure to merge when we can quickly revert if there are problems. |
Kobzol
force-pushed
the
assigned-prs-in-memory
branch
from
b7c8cd8 to
fc3b6f9
Compare
Assignment based on review preferences is not working currently.
Kobzol
force-pushed
the
assigned-prs-in-memory
branch
from
fc3b6f9 to
c4a6179
Compare
|
I rebased the PR and went through the changes again. I hope that this shouldn't break stuff, but one never knows. After merging, I'll monitor the logs and triagebot's state. The bors queue is borked anyway 😆 |
|
Hmm, off to a good start.
While loading the initial list of PRs. I only renamed that function in this PR though, so pretty sure that it is a GH problem (maybe because we have more open PRs now due to the bors queue being stuck?). Otherwise it seems to work though. |
This PR stops using
assigned_prscolumn in thereview_prefstable, and remembers PR assignments in-memory instead.