Repository navigation
feat(self-host): frontend MDC verify-and-cache pipeline (gh-8749) #9057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 21 commits
83c48dc
01e996c
610985b
6077ce2
514079f
d56ed8c
ea22e63
fa5d981
fc6f9cf
b07bb03
a7b7e1c
30f646e
81f56fe
a3f380f
5b1c50b
2be7de5
b9f8586
eeabebf
f2d8e1d
6fbe4a1
890aa3a
76e4b46
b7d06f4
2030aa0
8e04cc3
0f4f667
ff79745
c147dd3
81b4358
53ee6c7
6b2a342
312c447
011320f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2024-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| use std::path::PathBuf; | ||
| use std::sync::Arc; | ||
|
|
||
| use crate::{ | ||
|
|
@@ -83,6 +84,8 @@ pub async fn run( | |
| model.namespace(), | ||
| model.namespace_prefix(), | ||
| ); | ||
| let local_model_path = | ||
| (!model.path().as_os_str().is_empty()).then(|| model.path().to_path_buf()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In here and the other places, why do you do |
||
| run_watcher( | ||
| distributed_runtime.clone(), | ||
| http_service.state().manager_clone(), | ||
|
|
@@ -94,6 +97,7 @@ pub async fn run( | |
| http_service.state().metrics_clone(), | ||
| chat_engine_factory.clone(), | ||
| prefill_load_estimator.clone(), | ||
| local_model_path, | ||
| ) | ||
| .await?; | ||
| http_service | ||
|
|
@@ -173,6 +177,7 @@ async fn run_watcher( | |
| metrics: Arc<crate::http::service::metrics::Metrics>, | ||
| chat_engine_factory: Option<ChatEngineFactoryCallback>, | ||
| prefill_load_estimator: Option<Arc<dyn dynamo_kv_router::PrefillLoadEstimator>>, | ||
| local_model_path: Option<PathBuf>, | ||
| ) -> anyhow::Result<()> { | ||
| let mut watch_obj = ModelWatcher::new( | ||
| runtime.clone(), | ||
|
|
@@ -184,6 +189,7 @@ async fn run_watcher( | |
| prefill_load_estimator, | ||
| metrics.clone(), | ||
| ); | ||
| watch_obj.set_local_model_path(local_model_path); | ||
| tracing::debug!("Waiting for remote model"); | ||
| let discovery = runtime.discovery(); | ||
| let discovery_stream = discovery | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -564,24 +564,20 @@ impl LocalModel { | |
| let Some(local_path) = cf.path().map(Path::to_path_buf) else { | ||
| continue; | ||
| }; | ||
| // Filename from the original path — HF cache symlinks would | ||
| // otherwise canonicalize to the LFS SHA1 and break downstream | ||
| // lookups by literal name (e.g. `parent.join("generation_config.json")`). | ||
| let Some(filename) = local_path | ||
| .file_name() | ||
| .and_then(|f| f.to_str()) | ||
| .map(|s| s.to_string()) | ||
| else { | ||
| continue; | ||
| }; | ||
| // Canonicalize so the handler serves the resolved HF blob, not the symlink. | ||
| let absolute = match fs::canonicalize(&local_path) { | ||
| let absolute = match std::path::absolute(&local_path) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the docs:
You deleted the comment about symlinks, does that mean we don't have symlinks here any more? This one seems like it might break things. |
||
| Ok(p) => p, | ||
| Err(err) => { | ||
| tracing::warn!( | ||
| path = %local_path.display(), | ||
| %err, | ||
| "failed to canonicalize self-host metadata path; skipping", | ||
| "failed to absolutize self-host metadata path; skipping", | ||
| ); | ||
| continue; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't check the hash is correct, it only checks that it looks like a hash.
I gather that's intended, but it's confusing. Could you check the shape when you check the value also?
What does that mean? Is it relevant here.