Skip to content

chore(ci): fix model path using mounted pvc location(wip) - #828

Closed
key4ng wants to merge 1 commit into
mainfrom
fix-model-path
Closed

key4ng wants to merge 1 commit into
mainfrom
fix-model-path

Conversation

@key4ng

@key4ng key4ng commented Mar 19, 2026 •

Copy link
Copy Markdown
Member

Description

Problem

Solution

Changes

Test Plan

Checklist
  • cargo +nightly fmt passes
  • cargo clippy --all-targets --all-features -- -D warnings passes
  • (Optional) Documentation updated
  • (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

  • Chores
    • Updated continuous integration and benchmark infrastructure configuration for improved model loading and caching efficiency across test environments.

Signed-off-by: key4ng <rukeyang@gmail.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@coderabbitai

coderabbitai Bot commented Mar 19, 2026 •

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0c4d3a02-6094-4db4-a6bb-394be6d815d3

📥 Commits

Reviewing files that changed from the base of the PR and between e231a73 and f3ae25d.

📒 Files selected for processing (2)
  • .github/workflows/nightly-benchmark.yml
  • .github/workflows/pr-test-rust.yml

📝 Walkthrough

Walkthrough

Updated GitHub Actions workflows to replace ROUTER_LOCAL_MODEL_PATH environment variable with HF_HOME across benchmark job invocations in nightly and PR test workflows. This standardizes model loading configuration to use Hugging Face's standard environment variable across multiple CI job variants.

Changes

Cohort / File(s) Summary
Nightly Benchmark Workflows
.github/workflows/nightly-benchmark.yml
Replaced ROUTER_LOCAL_MODEL_PATH="/raid/models" with HF_HOME="/models" in single-worker, multi-worker, and single-worker-h200 job pytest invocations.
PR Test Rust Workflows
.github/workflows/pr-test-rust.yml
Replaced ROUTER_LOCAL_MODEL_PATH="/home/ubuntu/models" with HF_HOME="/models" in benchmarks and go-bindings-e2e job pytest invocations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

ci

Suggested reviewers

  • CatherineSue
  • slin1237
  • XinyueZhang369

Poem

🐰 Hop, hop, through the workflows we go,
ROUTER_LOCAL swaps for HF_HOME's glow,
Models now load with a standard care,
Benchmarks will run with variables fair! ✨

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-model-path
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@mergify

mergify Bot commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Closing this PR because the branch name fix-model-path does not follow the naming convention.

To fix this, please rename your branch locally, push the new branch, and open a new PR:

git branch -m <new-branch-name>
git push origin -u <new-branch-name>

@mergify mergify Bot closed this Mar 19, 2026
@mergify

mergify Bot commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Hi @key4ng, the branch fix-model-path does not follow our naming convention.

Please use one of the following formats:

  • <type>/<description> — e.g. feat/add-auth, fix/null-pointer, dependabot/cargo/pyo3-0.28.1
  • <username>/<description> — e.g. changsu/fix-routing

Allowed types: feat, fix, chore, docs, refactor, test, ci, perf

Note: PRs with non-conforming branch names will be auto-closed. Please follow the naming convention for all branches.

@key4ng
key4ng deleted the fix-model-path branch March 19, 2026 19:27
@github-actions github-actions Bot added the ci CI/CD configuration changes label Mar 19, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f3ae25d073

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


E2E_RUNTIME=${{ matrix.variant.runtime }} \
ROUTER_LOCAL_MODEL_PATH="/raid/models" \
HF_HOME="/models" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve ROUTER_LOCAL_MODEL_PATH in benchmark invocation

Switching this env var to HF_HOME breaks the local-model path plumbing used by the benchmark fixtures: get_model_spec() only checks ROUTER_LOCAL_MODEL_PATH when resolving model paths (e2e_test/infra/model_specs.py), and benchmark command construction only mounts/uses local tokenizer directories when ROUTER_LOCAL_MODEL_PATH is set (e2e_test/benchmarks/conftest.py). In this configuration, tests fall back to Hub model IDs instead of the mounted PVC model tree, which can cause nightly/CI benchmark runs to download from Hugging Face (or fail on gated/offline environments) instead of using local models.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI/CD configuration changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant