Skip to content

Feat; add releases to the benchmark_request queue #2192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 7, 2025
Merged

Feat; add releases to the benchmark_request queue #2192

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e7770f
Feat; add releases to the benchmark_request queue
Jamesbarford Jul 7, 2025
60d7321
PR Feedback; use Option<...> type only and limit to 20 results
Jamesbarford Jul 7, 2025
069f182
PR Feedback; take last 20 successfully parsed releases
Jamesbarford Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 26 additions & 36 deletions site/src/job_queue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{
};

use crate::load::{partition_in_place, SiteCtxt};
use chrono::{DateTime, NaiveDate, Timelike, Utc};
use chrono::{DateTime, NaiveDate, Utc};
use database::{BenchmarkRequest, BenchmarkRequestStatus, BenchmarkRequestType};
use hashbrown::HashSet;
use parking_lot::RwLock;
Expand Down Expand Up @@ -48,20 +48,16 @@ async fn create_benchmark_request_master_commits(
/// `static.rust-lang.org/dist/2025-06-26/channel-rust-1.89-beta.toml`
/// `static.rust-lang.org/dist/2025-06-26/channel-rust-1.89.0-beta.toml`
/// `static.rust-lang.org/dist/2025-06-26/channel-rust-1.89.0-beta.2.toml`
fn parse_release_string(url: &str) -> anyhow::Result<Option<(String, DateTime<Utc>)>> {
fn parse_release_string(url: &str) -> Option<(String, DateTime<Utc>)> {
static VERSION_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"(\d+\.\d+\.\d+)").unwrap());

// Grab ".../YYYY-MM-DD/FILE.toml" components with Path helpers.
let file = Path::new(url)
.file_name()
.and_then(|n| n.to_str())
.ok_or_else(|| anyhow::anyhow!("URL lacks a file name"))?;
let file = Path::new(url).file_name().and_then(|n| n.to_str())?;

let date_str = Path::new(url)
.parent()
.and_then(Path::file_name)
.and_then(|n| n.to_str())
.ok_or_else(|| anyhow::anyhow!("URL lacks a date segment"))?;
.and_then(|n| n.to_str())?;

// No other beta releases are recognized as toolchains.
//
Expand All @@ -73,30 +69,30 @@ fn parse_release_string(url: &str) -> anyhow::Result<Option<(String, DateTime<Ut
//
// Which should get ignored for now, they're not consumable via rustup yet.
if file.contains("beta") && file != "channel-rust-beta.toml" {
return Ok(None);
return None;
}

// Parse the YYYY-MM-DD segment and stamp it with *current* UTC time.
let naive = NaiveDate::parse_from_str(date_str, "%Y-%m-%d")?;
let now = Utc::now();
let published = naive
.and_hms_nano_opt(now.hour(), now.minute(), now.second(), now.nanosecond())
.expect("valid HMS")
.and_local_timezone(Utc)
.single()
.unwrap();

// Special-case the rolling beta channel.
if file == "channel-rust-beta.toml" {
return Ok(Some((format!("beta-{date_str}"), published)));
}
if let Ok(naive) = NaiveDate::parse_from_str(date_str, "%Y-%m-%d") {
let published = naive
.and_hms_opt(0, 0, 0)
.expect("valid HMS")
.and_local_timezone(Utc)
.single()
.unwrap();

// Special-case the rolling beta channel.
if file == "channel-rust-beta.toml" {
return Some((format!("beta-{date_str}"), published));
}

// Otherwise pull out a semver like "1.70.0" and return it.
if let Some(cap) = VERSION_RE.captures(file).and_then(|m| m.get(1)) {
return Ok(Some((cap.as_str().to_owned(), published)));
// Otherwise pull out a semver like "1.70.0" and return it.
if let Some(cap) = VERSION_RE.captures(file).and_then(|m| m.get(1)) {
return Some((cap.as_str().to_owned(), published));
}
}

Ok(None)
None
}

/// Store the latest release commits or do nothing if all of them are
Expand All @@ -111,8 +107,8 @@ async fn create_benchmark_request_releases(
// TODO; delete at some point in the future
let cutoff: chrono::DateTime<Utc> = chrono::DateTime::from_str("2025-06-01T00:00:00.000Z")?;

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) {
Copy link
Member

@Kobzol Kobzol Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@Jamesbarford Jamesbarford Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@Jamesbarford Jamesbarford Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

069f182 I've changed it 👍

Copy link
Member

@Kobzol Kobzol Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 👍

if let Some((name, date_time)) = parse_release_string(release_string) {
if date_time >= cutoff {
let release_request = BenchmarkRequest::create_release(
&name,
Expand Down Expand Up @@ -313,18 +309,13 @@ mod tests {
/// Helper: unwrap the Option, panic otherwise.
fn tag(url: &str) -> String {
parse_release_string(url)
.unwrap() // anyhow::Result<_>
.expect("Some") // Option<_>
.0 // take the tag
}

/// Helper: unwrap the DateTime and keep only the YYYY-MM-DD part
fn day(url: &str) -> NaiveDate {
parse_release_string(url)
.unwrap()
.expect("Some")
.1
.date_naive()
parse_release_string(url).expect("Some").1.date_naive()
}

fn days_ago(day_str: &str) -> chrono::DateTime<Utc> {
Expand Down Expand Up @@ -749,7 +740,6 @@ mod tests {
assert!(parse_release_string(
"static.rust-lang.org/dist/2016-05-31/channel-rust-nightly.toml"
)
.unwrap()
.is_none());

// versioned-beta artefacts are skipped too
Expand All @@ -759,7 +749,7 @@ mod tests {
"static.rust-lang.org/dist/2025-06-26/channel-rust-1.89.0-beta.2.toml",
] {
assert!(
parse_release_string(should_ignore).unwrap().is_none(),
parse_release_string(should_ignore).is_none(),
"{should_ignore} should be ignored"
);
}
Expand Down
Loading