Address reviewer findings on PR #5748: 4 release-path bugs

Round-trip with the reviewer.py 12-persona pass surfaced four real
issues. Fix all four in this PR so the new Docker release path is
self-consistent.

1. docker/smoke_test.py used `import xformers` unconditionally, which
   guarantees a failure on arm64 (built with `[huggingface]` extras to
   skip xformers since it has no aarch64 cu128 wheel). Wrap the import
   in try/except so the same smoke script validates both arches.

2. unsloth/_gpu_init.py forced `TORCHINDUCTOR_COMPILE_THREADS=1` before
   `import unsloth_zoo`, but `patch_torch_compile` in unsloth_zoo main
   pops that env var in non-debug mode. After unsloth_zoo init the
   guard was effectively undone, so cgroup-pinned `docker --gpus
   '"device=N"'` containers still spawned the Inductor subprocess pool
   that cannot enumerate the GPU. Set `torch._inductor.config.
   compile_threads = 1` directly post-import-torch and re-populate the
   env var so `determine_compile_threads()` in the zoo options dict
   also returns 1, regardless of whether the zoo-side fix from PR #694
   has shipped yet.

3. docker-publish.yml UNSLOTH_REF build-arg defaulted to `'main'` for
   tag pushes and scheduled runs, so a `v1.2.3` release image would
   contain whatever `main` happened to be at build time, not v1.2.3.
   Pick the tag's `github.ref_name` for tag events and `github.sha`
   for branch/schedule events.

4. The smoke-test job pulled `:latest` regardless of which tag the
   merge job had just published, so tag/schedule/sha publishes were
   never actually validated. Re-run docker/metadata-action with the
   same config the merge job used, then smoke-test the first tag from
   its output.

All four changes are gated and backwards-compatible.
This commit is contained in:
danielhanchen 2026-05-24 14:05:36 +00:00
commit 29a6bde4d1
54 changed files with 107 additions and 8 deletions

View file

@ -119,7 +119,12 @@ jobs:
CUDA_VERSION=12.8.1
UBUNTU_VERSION=24.04
PYTHON_VERSION=3.12
UNSLOTH_REF=${{ github.event.inputs.unsloth_ref || 'main' }}
# Workflow-dispatch: honour the explicit input. Tag pushes:
# bake the tag's source ref (e.g. v1.2.3) so the published
# tag image actually contains that release. Branch pushes and
# scheduled runs: bake the triggering commit SHA. Falls back
# to `main` for any other event class.
UNSLOTH_REF=${{ github.event.inputs.unsloth_ref || (startsWith(github.ref, 'refs/tags/') && github.ref_name) || github.sha || 'main' }}
UNSLOTH_ZOO_REF=${{ github.event.inputs.unsloth_zoo_ref || 'main' }}
# Stash the per-arch digest as an artifact for the merge job to pick up.
@ -203,9 +208,30 @@ jobs:
timeout-minutes: 20
steps:
- uses: actions/checkout@v4
# Re-compute the tag list deterministically from the same metadata-action
# config the merge job used, so tag/schedule/SHA runs pull the image
# they just published instead of an unrelated `:latest` from a prior run.
- name: Resolve published tag
id: meta
uses: docker/metadata-action@v5
with:
images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
tags: |
type=raw,value=latest,enable={{is_default_branch}}
type=ref,event=tag
type=schedule,pattern=nightly
type=sha,prefix=sha-,format=short
- name: Pull and smoke-test
run: |
docker pull ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest
docker run --rm --gpus all \
${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest \
python /workspace/smoke_test.py
# Use the first tag from the metadata output -- that is the image we
# just published. Falls back to :latest only when the metadata is
# empty (defensive; should not happen on default-branch runs).
TAG="$(jq -r '.tags[0] // ""' <<<"$DOCKER_METADATA_OUTPUT_JSON")"
if [ -z "$TAG" ]; then
TAG="${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest"
fi
echo "smoke-testing $TAG"
docker pull "$TAG"
docker run --rm --gpus all "$TAG" python /workspace/smoke_test.py