[Chore][CI]: Change dst for K3 nightly comprehensive results#2958
[Chore][CI]: Change dst for K3 nightly comprehensive results#2958sammshen merged 1 commit intoLMCache:devfrom
Conversation
The GitHub PAT for the main repo expired, causing nightly baseline uploads to fail. Switch the upload target to the dedicated LMCache/LMCache-CI repository instead of pushing to benchmarks-main on the main LMCache repo. Signed-off-by: Samuel Shen <slshen@uchciago.edu>
There was a problem hiding this comment.
Code Review
This pull request updates the baseline upload script to push benchmark results to a dedicated CI repository (LMCache/LMCache-CI) instead of the main repository, including logic for GITHUB_TOKEN authentication. A review comment suggests removing stderr suppression during git push operations to improve debuggability of CI failures, as hiding error messages complicates troubleshooting and constitutes technical debt.
| if ! git push origin "HEAD:${CI_BRANCH}" 2>/dev/null; then | ||
| echo "[WARN] Normal push failed, force-pushing..." | ||
| git push "$PUSH_URL" +HEAD:benchmarks-main 2>/dev/null || { | ||
| echo "[ERROR] Failed to push baselines to benchmarks-main" | ||
| git push origin "+HEAD:${CI_BRANCH}" 2>/dev/null || { |
There was a problem hiding this comment.
Suppressing stderr with 2>/dev/null during git push makes it difficult to diagnose failures in CI. If the push fails due to authentication issues (e.g., an expired or insufficient PAT), permission errors, or network problems, the actual error message will be hidden, leaving only the generic error message. Removing the redirection allows for better debuggability of CI failures.
| if ! git push origin "HEAD:${CI_BRANCH}" 2>/dev/null; then | |
| echo "[WARN] Normal push failed, force-pushing..." | |
| git push "$PUSH_URL" +HEAD:benchmarks-main 2>/dev/null || { | |
| echo "[ERROR] Failed to push baselines to benchmarks-main" | |
| git push origin "+HEAD:${CI_BRANCH}" 2>/dev/null || { | |
| if ! git push origin "HEAD:${CI_BRANCH}"; then | |
| echo "[WARN] Normal push failed, force-pushing..." | |
| git push origin "+HEAD:${CI_BRANCH}" || { |
References
- The style guide emphasizes focusing on technical debt and future maintainability. Hiding error messages in CI scripts is a form of technical debt that hinders maintainability and debuggability. (link)
Allows a fine-grained PAT that never expires for the CI machine to push results to.
What this PR does / why we need it:
Special notes for your reviewers:
If applicable:
Note
Medium Risk
Moderate risk: changes the checkout/push flow and destination repo for baseline artifacts, which could break nightly uploads if credentials/branch setup differ or cloning fails.
Overview
Nightly baseline upload now publishes rolling
benchmarks/long_doc_qaartifacts to a dedicated repo (LMCache/LMCache-CI) onbenchmarks-maininstead of pushing back to the main LMCache repository.The script replaces the prior
origin/benchmarks-mainfetch/worktree logic and dynamic push URL construction with agit cloneof the CI repo (creating an orphan branch if missing) and pushes viaoriginwith a normal/force-push fallback usingGITHUB_TOKENwhen available.Reviewed by Cursor Bugbot for commit 0d63c99. Bugbot is set up for automated code reviews on this repo. Configure here.