Skip to content

Add temporary CI-image Dockerfile variant with sglang PR 30499 - #1601

Closed
yushengsu-thu wants to merge 1 commit into
mainfrom
ci-image-qwen35-gdn
Closed

yushengsu-thu wants to merge 1 commit into
mainfrom
ci-image-qwen35-gdn

Conversation

@yushengsu-thu

Copy link
Copy Markdown
Collaborator

Builds sglang from pull/30499/head (GDN in_proj_ba LoRA support) so the Qwen3.5 LoRA CI in #1597 can run before the sglang PR merges. Throwaway branch; delete after sglang-miles picks up the PR.

Builds sglang from pull/30499/head (GDN in_proj_ba LoRA support) so the
Qwen3.5 LoRA CI in #1597 can run before the sglang PR
merges. Throwaway branch; delete after sglang-miles picks up the PR.

Signed-off-by: Yusheng Su <yushengsu.thu@gmail.com>
Copilot AI review requested due to automatic review settings July 8, 2026 21:53

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new Dockerfile (docker/Dockerfile.qwen35-gdn) to build a specialized CI image containing various machine learning dependencies, including SGLang, Megatron-LM, and FlashQLA. The review feedback suggests improving the robustness of the GitHub API asset-fetching script by explicitly handling API rate limit errors instead of failing silently when the 'assets' key is missing.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +66 to +69
| python3 -c "import sys,json,subprocess,os; w='/tmp/wheels'; \
[subprocess.run(['curl','-fSL','-o',os.path.join(w,a['name']),a['browser_download_url']],check=True) \
for a in json.load(sys.stdin).get('assets',[]) \
if a['name'].endswith(('.whl', '.tar.gz')) and not os.path.exists(os.path.join(w,a['name']))]" && \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When fetching the wheels release from the GitHub API, if the API rate limit is exceeded (which is common in CI environments using shared IPs), GitHub returns a 403 Forbidden response with a JSON body containing a message field but no assets field.

Currently, json.load(sys.stdin).get('assets', []) will silently return an empty list [] on rate limit errors, causing the build to succeed in this step but fail later with a cryptic pip install error due to missing wheels.

We can make this more robust by checking for the presence of the assets key and exiting with a clear error message if it is missing.

    | python3 -c "import sys,json,subprocess,os; w='/tmp/wheels'; d=json.load(sys.stdin); \
sys.exit('GitHub API Error: ' + str(d.get('message', d))) if 'assets' not in d else \
[subprocess.run(['curl','-fSL','-o',os.path.join(w,a['name']),a['browser_download_url']],check=True) \
 for a in d['assets'] \
 if a['name'].endswith(('.whl', '.tar.gz')) and not os.path.exists(os.path.join(w,a['name']))]" && \

@yushengsu-thu

Copy link
Copy Markdown
Collaborator Author

Not for merging — this branch only exists as the build source for the temporary CI image radixark/miles:qwen35-gdn (sglang PR sgl-project/sglang#30499 baked in) used by #1597's CI. The branch will be deleted once the sglang PR lands in sglang-miles and the dev image rebuilds.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants