Skip to content

Add cluster-wide git_http_version config for intermittent git smart-HTTP failures - #376

Merged
ishandhanani merged 2 commits into
mainfrom
idhanani/git-clone-http-version
Sep 3, 2026
Merged

ishandhanani merged 2 commits into
mainfrom
idhanani/git-clone-http-version

Conversation

@ishandhanani

Copy link
Copy Markdown
Collaborator

Summary

Some clusters see intermittent git smart-HTTP failures negotiating HTTP/2 against github.com -- stalls, or truncated responses git misreports as could not read Username auth-prompt failures -- on certain network paths. Observed live on NVIDIA/InferenceMAX PR #271, on both a login host and its compute nodes, across three separate git operations (clone, fetch, and checkout of a partial clone, which does its own lazy blob fetch for missing objects).

  • New srtctl.core.config.git_clone_command_prefix(), reading an optional git_http_version setting from srtslurm.yaml (e.g. "HTTP/1.1").
  • Wired into every git clone/fetch on the real sbatch path: the dynamo-from-source install scripts in core/schema.py (highest-impact site -- used by any recipe with dynamo.install: true, not just one feature), and the srtlog post-processing clone (cli/mixins/postprocess_stage.py).
  • --bash dev-mode (render/direct_stages/) can't read srtslurm.yaml (deliberately stdlib-only, no srtctl.core imports) -- applies http.version=HTTP/1.1 unconditionally there instead of leaving it unfixed.
  • Default (git_http_version unset) is unchanged -- plain git, no behavior change for clusters that don't need this.

Test plan

  • Full suite: 1544 passed, 2 skipped -- the 2 unrelated failures (test_fingerprint.py, test_apply_mock.py) are pre-existing macOS-only platform gaps (os.sched_getaffinity doesn't exist on macOS), confirmed by reproducing the same failures on a clean origin/main checkout.
  • Live validation pending on NVIDIA/InferenceMAX PR feat(vllm): support mixed DP launch modes #271, which hit this exact failure mode.

…git smart-HTTP failures

Some clusters see intermittent git smart-HTTP failures negotiating HTTP/2
against github.com -- stalls, or truncated responses git misreports as
"could not read Username" auth-prompt failures -- on certain network
paths. Observed on NVIDIA/InferenceMAX PR #271 on both a login host and
its compute nodes, across three separate git operations (clone, fetch,
and checkout of a partial clone, which does its own lazy blob fetch).

Add srtctl.core.config.git_clone_command_prefix(), reading a new
git_http_version srtslurm.yaml setting (e.g. "HTTP/1.1"), and wire it
into every git clone/fetch srtctl performs on the real sbatch path:
the dynamo-from-source install scripts (core/schema.py, the highest-impact
site -- used by any recipe with dynamo.install: true) and the srtlog
post-processing clone (cli/mixins/postprocess_stage.py).

--bash dev-mode (render/direct_stages/) can't read srtslurm.yaml (deliberately
stdlib-only, no srtctl.core imports) -- applies the same http.version=HTTP/1.1
guard unconditionally instead, rather than leaving it unfixed.

Default (git_http_version unset) is unchanged: plain `git`, no behavior
change for clusters that don't need this.
@ishandhanani
ishandhanani merged commit be21dc5 into main Sep 3, 2026
6 checks passed
ishandhanani added a commit that referenced this pull request Sep 3, 2026
…eclared) (#377)

#376 added git_clone_command_prefix() reading git_http_version via
get_srtslurm_setting(), but never declared the key on ClusterConfig.
srtslurm.yaml is schema-validated, and an unknown top-level key fails
that validation and silently drops every cluster default for the whole
config, not just the unrecognized key -- confirmed live on
NVIDIA/InferenceMAX PR #271: "Failed to load or validate srtslurm.yaml:
{'git_http_version': ['Unknown field.']}", which also broke model_paths
resolution as collateral damage (FileNotFoundError on a literal
"deepseek-v4-pro" path instead of the aliased /scratch/... path).

Same lesson TestHostSetup.test_cluster_schema_accepts_the_key already
captured for an earlier field -- #376 needed the same test and didn't
have it.
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.

1 participant