Feat; add releases to the benchmark_request queue #2192
Conversation
site/src/job_queue.rs
Outdated
|
|
||
| for release_string in releases.lines() { | ||
| if let Some((name, date_time)) = parse_release_string(release_string)? { | ||
| for release_string in releases.lines().rev().take(20) { |
There was a problem hiding this comment.
This is a bit different than we need - here you load the last 20 lines, but we need the last 20 release artifacts (imagine e.g. that we had 20 lines with mostly nightly releases, then we'd take only a few actual release artifacts). Check how the parse_published... function was used, we can just copy paste it.
There was a problem hiding this comment.
My previous method, I think, was implementing what is there currently. The current function parses all of the list then makes a decision if I understand correctly;
// site/src/load.rs L:236
let artifacts: Vec<_> = artifact_list
.lines()
.rev()
.filter_map(parse_published_artifact_tag) // Will parse every item in the list
.take(20)
.filter(|artifact| {
!tested_artifacts.contains(artifact.as_str())
&& !in_progress_tagged_artifacts.contains(artifact.as_str())
})
.collect();So I think what I was doing previously was fine?
There was a problem hiding this comment.
069f182 I've changed it 👍
There was a problem hiding this comment.
Let's say that we take the last 3 (instead of 20) releases, to simplify this. If we have this input:
nightly-A
1.10.0
nightly-B
1.11.0
1.12.0
nightly-C
nightly-D
then
for release_string in releases.lines().rev().take(3) will iterate over nightly-D, nightly-C and 1.12.0, and extract only 1.12.0.
What we want instead is to parse 1.12.0, 1.11.0 and 1.10.0.
artifact_list
.lines()
.rev()
.filter_map(parse_published_artifact_tag) // Will parse every item in the list
.take(3)goes through the lines in reverse order, keeps only the release artifact lines in the iterator chain, and then takes the first three. That's exactly what we want.
The .collect() is not needed, but otherwise the new logic is fine 👍
There was a problem hiding this comment.
Thanks! I tested it locally, and it seems that it works fine. I also noticed that one of the benchmark requests is already marked as being in progress.. we should remove this logic from the code, even though it's WIP, it will mess up with out invariants later, because we haven't enqueued any jobs yet, so we might have to fix up the DB manually.
benchmark_requestqueue