central_systest_blobs: move blobs.json storage out of git, into GCS#13946
Conversation
PR SummaryMedium Risk Overview CI is updated to support this. The Reviewed by Cursor Bugbot for commit 42786bd. Bugbot is set up for automated code reviews on this repo. Configure here. |
723b1eb to
b83d810
Compare
b83d810 to
320474b
Compare
1c25347 to
5707677
Compare
5c06049 to
18f3a32
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware resolved 1 discussion.
Reviewable status: 0 of 7 files reviewed, all discussions resolved (waiting on idan-starkware and nimrod-starkware).
5707677 to
dd4db61
Compare
18f3a32 to
8b02186
Compare
nimrod-starkware
left a comment
There was a problem hiding this comment.
@nimrod-starkware reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: 4 of 7 files reviewed, 1 unresolved discussion (waiting on dorimedini-starkware and idan-starkware).
crates/central_systest_blobs/src/cende_blob_regression_test.rs line 60 at r4 (raw file):
.parse() .unwrap() }
what is the ID for the first time? how do you store the json for the first time?
Code quote:
fn current_generation() -> usize {
let path = current_dir().unwrap().join(BLOBS_GENERATION_FILE);
fs::read_to_string(path.clone())
.unwrap_or_else(|error| panic!("Failed to read file {path:?}: {error}"))
.trim()
.parse()
.unwrap()
}8b02186 to
f452e4d
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware made 1 comment.
Reviewable status: 4 of 8 files reviewed, 1 unresolved discussion (waiting on idan-starkware and nimrod-starkware).
crates/central_systest_blobs/src/cende_blob_regression_test.rs line 60 at r4 (raw file):
Previously, nimrod-starkware wrote…
what is the ID for the first time? how do you store the json for the first time?
started 0, then i ran the test with UPDATE_EXPECT=1, the JSON was pushed to GCS with ID 1.
f452e4d to
978982a
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware made 1 comment.
Reviewable status: 2 of 8 files reviewed, 2 unresolved discussions (waiting on idan-starkware and nimrod-starkware).
crates/central_systest_blobs/src/cende_blob_regression_test.rs line 240 at r6 (raw file):
}; Client::new(client_config) }
@idan-starkware WDYT of this?
Code quote:
async fn gcs_client() -> Client {
// Try ADC first (CI service accounts, or `gcloud auth application-default login`).
// Fall back to user credentials from `gcloud auth login`.
let client_config = if let Ok(config) = ClientConfig::default().with_auth().await {
config
} else {
let account = String::from_utf8(
std::process::Command::new("gcloud")
.args(["config", "get-value", "account"])
.output()
.expect("failed to run `gcloud config get-value account`")
.stdout,
)
.expect("gcloud output is not UTF-8")
.trim()
.to_string();
let home = std::env::var("HOME").expect("HOME not set");
let creds_path = format!("{home}/.config/gcloud/legacy_credentials/{account}/adc.json");
let creds = google_cloud_auth::credentials::CredentialsFile::new_from_file(creds_path)
.await
.expect("Failed to load gcloud user credentials. Run `gcloud auth login`.");
ClientConfig::default()
.with_credentials(creds)
.await
.expect("Failed to create GCS client config. Did you run `gcloud auth login`?")
};
Client::new(client_config)
}
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 1 file.
Reviewable status: 2 of 8 files reviewed, 2 unresolved discussions (waiting on idan-starkware and nimrod-starkware).
978982a to
e911f8d
Compare
e911f8d to
f502321
Compare
dd4db61 to
8b6b28d
Compare
184fe63 to
7a5d52f
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware resolved 1 discussion.
Reviewable status: 1 of 8 files reviewed, 1 unresolved discussion (waiting on idan-starkware and nimrod-starkware).
7a5d52f to
42786bd
Compare
nimrod-starkware
left a comment
There was a problem hiding this comment.
@nimrod-starkware made 1 comment and resolved 1 discussion.
Reviewable status: 1 of 8 files reviewed, 1 unresolved discussion (waiting on dorimedini-starkware and idan-starkware).
crates/central_systest_blobs/src/cende_blob_regression_test.rs line 238 at r12 (raw file):
} } next_generation
Why isnt it guaranteed that next_generation is free?
Code quote:
let mut next_generation = current_generation() + 1;
loop {
match fetch_raw_blobs_at_generation(client, next_generation).await {
Ok(_) => next_generation += 1,
Err(GcsError::Response(ErrorResponse { code: GCS_ERROR_CODE_NOT_FOUND, .. })) => break,
Err(GcsError::HttpClient(error))
if error.status() == Some(http::StatusCode::NOT_FOUND) =>
{
break;
}
Err(e) => panic!("Failed to fetch blobs at generation {next_generation}: {e}"),
}
}
next_generation
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware made 1 comment.
Reviewable status: 1 of 8 files reviewed, 1 unresolved discussion (waiting on idan-starkware and nimrod-starkware).
crates/central_systest_blobs/src/cende_blob_regression_test.rs line 238 at r12 (raw file):
Previously, nimrod-starkware wrote…
Why isnt it guaranteed that next_generation is free?
say you have branches A->B->main.
all are currently in generation X.
on A you make some changes, and you need to iterate a bit until it works, so when you're done you're at generation X+7.
then you checkout B and realize you still need to regenerate the blobs.
If I don't check for next available slot, fixing on B will end up with generation X+1 instead of X+8
nimrod-starkware
left a comment
There was a problem hiding this comment.
@nimrod-starkware reviewed 5 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: 6 of 8 files reviewed, all discussions resolved (waiting on idan-starkware).
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 5 files and all commit messages.
Reviewable status: 6 of 8 files reviewed, all discussions resolved (waiting on idan-starkware).
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 2 files.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on idan-starkware).

No description provided.