Repository navigation
[OEP] Propose optional Dragonfly model downloads - #1205
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThis PR adds OEP 0012, a provisional proposal for optional Dragonfly transport for Hugging Face ChangesDragonfly transport proposal
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Other Merge Risk: ⚪ Minimal · up to This proposal keeps direct downloads as the default. No issue requiring resolution before merge was established. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The proposal does not enable new runtime behavior and retains direct downloads as the default. It establishes important integrity, credential-isolation, and recovery requirements, but their implementation and deployment qualification remain outstanding. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Reviewed this OEP in depth — no issues found.
Verified during review:
- PR description follows the template with all required sections filled.
- OEP number 0012 is unused;
oep.yamlmetadata (status, stage, metrics) is consistent with the README and with merged OEP precedent; the four metrics listed match the Observability table exactly. - The table of contents matches the document headings one-to-one.
- In-repo references are accurate: the
HFDownloadFunccallback shape matchesdirectHfSource.download(pkg/modelagent/gopher_hf_source.go), the claim thatremoveInvalidFilescannot remove files absent from a new manifest matches the implementation inpkg/modelagent/hf_snapshot_validation.go, themodel_agent_download_transport_*metric names follow the existingmodel_agent_prefix convention, andAlwaysDownload/ReuseIfExists/PerNodematch the v1beta1 API enums. - External pins check out: client tag v1.5.7 resolves to
ada71793…and server tag v2.5.2 todbc47411…as stated; the v1.5.7 Hugging Face backend does unconditionally installNoVerifierfor origin TLS; dfget v1.5.7 provides--hf-revision,DFGET_HF_TOKEN,DFGET_HF_BASE_URL, and--transfer-from-dfdaemon; and the native metrics do label vectors with the raw task tag, supporting the cardinality gate.
Severity counts: 🔴 0, 🟡 0, 🟣 0.
What this PR does
Adds OEP-0012 proposing optional Dragonfly transport for PerNode Hugging Face model downloads.
Defines immutable snapshot identity, credential partitioning, durable ownership and recovery, endpoint normalization, verification, fallback, observability, and rollout requirements.
Dragonfly remains opt-in. Default direct downloads and existing CRD APIs remain unchanged. This PR contains the proposal, not the implementation.
Why we need it
Concurrent downloads across nodes repeatedly fetch identical model files from the origin. Optional peer-to-peer distribution can reduce origin traffic and improve rollout latency without changing the serving storage layout.
The proposal also establishes security and qualification requirements before alpha release, explicitly excluding unpatched Dragonfly client v1.5.7.
Fixes: N/A — no linked issue.
How to test
Documentation-only change. Validation completed:
The OEP specifies future unit and end-to-end qualification tests. The full make test suite was not run.
Checklist
make testpasses locallySummary by CodeRabbit