Skip to content

central_systest_blobs: move blobs.json storage out of git, into GCS#13946

Merged
dorimedini-starkware merged 1 commit intomainfrom
04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs
May 5, 2026
Merged

central_systest_blobs: move blobs.json storage out of git, into GCS#13946
dorimedini-starkware merged 1 commit intomainfrom
04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs

Conversation

@dorimedini-starkware
Copy link
Copy Markdown
Collaborator

No description provided.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

Copy link
Copy Markdown
Collaborator Author

dorimedini-starkware commented May 3, 2026

@dorimedini-starkware dorimedini-starkware self-assigned this May 3, 2026
@dorimedini-starkware dorimedini-starkware marked this pull request as ready for review May 3, 2026 09:13
@cursor
Copy link
Copy Markdown

cursor Bot commented May 3, 2026

PR Summary

Medium Risk
Introduces an external dependency (GCS) into the regression test and CI via new auth/secret requirements, so failures can come from credentials, network, or object-versioning behavior rather than code changes.

Overview
Moves systest blob fixtures out of git and into GCS. The central_systest_blobs regression test now fetches the expected blobs.json from a GCS bucket keyed by a tracked generation, and in UPDATE_EXPECT mode uploads a new generation (without overwriting existing objects) and updates resources/blob_file_generation.

CI is updated to support this. The run-tests GitHub Actions job now authenticates to Google Cloud using SYSTEST_BLOBS_READ_ACCESS_KEY before running tests, and the crate adds google-cloud-storage/http (plus testing features/deps adjustments) to support the new GCS-based workflow.

Reviewed by Cursor Bugbot for commit 42786bd. Bugbot is set up for automated code reviews on this repo. Configure here.

@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs branch from 723b1eb to b83d810 Compare May 3, 2026 09:17
Comment thread crates/central_systest_blobs/src/cende_blob_regression_test.rs
@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs branch from b83d810 to 320474b Compare May 3, 2026 09:27
Comment thread crates/central_systest_blobs/src/cende_blob_regression_test.rs Outdated
@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-apollo_consensus_orchestrator_add_deserialize_for_testing branch from 1c25347 to 5707677 Compare May 3, 2026 10:07
@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs branch 2 times, most recently from 5c06049 to 18f3a32 Compare May 3, 2026 10:12
Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@dorimedini-starkware resolved 1 discussion.
Reviewable status: 0 of 7 files reviewed, all discussions resolved (waiting on idan-starkware and nimrod-starkware).

@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-apollo_consensus_orchestrator_add_deserialize_for_testing branch from 5707677 to dd4db61 Compare May 3, 2026 10:25
@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs branch from 18f3a32 to 8b02186 Compare May 3, 2026 10:25
Copy link
Copy Markdown
Contributor

@nimrod-starkware nimrod-starkware left a comment

Choose a reason for hiding this comment

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

@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()
}

@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs branch from 8b02186 to f452e4d Compare May 3, 2026 12:36
Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@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.

Comment thread crates/central_systest_blobs/src/cende_blob_regression_test.rs
@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs branch from f452e4d to 978982a Compare May 3, 2026 13:09
Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@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)
}

Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@dorimedini-starkware reviewed 1 file.
Reviewable status: 2 of 8 files reviewed, 2 unresolved discussions (waiting on idan-starkware and nimrod-starkware).

@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs branch from 978982a to e911f8d Compare May 3, 2026 14:13
@dorimedini-starkware dorimedini-starkware changed the base branch from 04-30-apollo_consensus_orchestrator_add_deserialize_for_testing to graphite-base/13946 May 3, 2026 15:44
@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs branch from e911f8d to f502321 Compare May 3, 2026 15:44
@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-central_systest_blobs_move_blobs.json_storage_out_of_git_into_gcs branch from 184fe63 to 7a5d52f Compare May 4, 2026 09:52
Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@dorimedini-starkware resolved 1 discussion.
Reviewable status: 1 of 8 files reviewed, 1 unresolved discussion (waiting on idan-starkware and nimrod-starkware).

Copy link
Copy Markdown
Contributor

@nimrod-starkware nimrod-starkware left a comment

Choose a reason for hiding this comment

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

@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

Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@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

Copy link
Copy Markdown
Contributor

@nimrod-starkware nimrod-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@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).

Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@dorimedini-starkware reviewed 5 files and all commit messages.
Reviewable status: 6 of 8 files reviewed, all discussions resolved (waiting on idan-starkware).

Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@dorimedini-starkware reviewed 2 files.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on idan-starkware).

@dorimedini-starkware dorimedini-starkware added this pull request to the merge queue May 5, 2026
Merged via the queue into main with commit b7ebc45 May 5, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants