Repository navigation
fix(model-manager): filter repo-id Diffusers files by index #9912
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
base: main
Are you sure you want to change the base?
Changes from all commits
7db0926
cf34a06
857efe5
dc61500
53dcba5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,14 +126,18 @@ def download_urls( | |
|
|
||
| paths = filter_files([x.path for x in self.files], variant, subfolder, subfolders) # all files in the model | ||
|
|
||
| repo_root = Path(self.name) if paths and paths[0].parts[0] == self.name else Path() | ||
|
|
||
| # Determine prefix for model_index.json check - only applies for single subfolder | ||
| prefix = "" | ||
| if subfolder and not subfolders: | ||
| prefix = f"{subfolder}/" | ||
|
|
||
| # the next step reads model_index.json to determine which subdirectories belong | ||
| # to the model (only for single subfolder case) | ||
| if Path(f"{prefix}model_index.json") in paths: | ||
| model_root = repo_root / prefix | ||
| model_index_path = model_root / "model_index.json" | ||
|
Collaborator
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. [P1] Authenticate the newly enabled model-index request Recognizing repository-prefixed paths now makes normal repo-id installs execute the index GET below, which uses a plain |
||
| if model_index_path in paths: | ||
|
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.
When a repository exposes an actual variant revision (for example AGENTS.md reference: invokeai/backend/AGENTS.md:L9-L9 Useful? React with 👍 / 👎. |
||
| url = hf_hub_url(self.id, filename="model_index.json", subfolder=str(subfolder) if subfolder else None) | ||
|
Member
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. On Windows, The line predates this PR, but this PR is what makes it reachable. |
||
| try: | ||
| resp = session.get(url, timeout=HF_METADATA_REQUEST_TIMEOUT_SECONDS) | ||
|
|
@@ -145,8 +149,8 @@ def download_urls( | |
| except RequestException as e: | ||
| raise ModelMetadataUnavailableError(f"Could not fetch model index for '{self.id}'") from e | ||
| submodels = resp.json() | ||
| paths = [Path(subfolder or "", x) for x in paths if Path(x).parent.as_posix() in submodels] | ||
| paths.insert(0, Path(f"{prefix}model_index.json")) | ||
| paths = [x for x in paths if x.relative_to(model_root).parent.as_posix() in submodels] | ||
|
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.
When installing AGENTS.md reference: invokeai/backend/AGENTS.md:L9-L9 Useful? React with 👍 / 👎. |
||
| paths.insert(0, model_index_path) | ||
|
|
||
| return [x for x in self.files if x.path in paths] | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| from pathlib import Path | ||
| from unittest.mock import Mock | ||
|
|
||
| from invokeai.backend.model_manager.metadata.metadata_base import HuggingFaceMetadata, RemoteModelFile | ||
|
|
||
|
|
||
| def test_repo_id_diffusers_download_uses_model_index_components() -> None: | ||
| session = Mock() | ||
| response = Mock(status_code=200) | ||
| response.json.return_value = { | ||
| "_class_name": "StableDiffusionPipeline", | ||
| "unet": ["diffusers", "UNet2DConditionModel"], | ||
| "vae": ["diffusers", "AutoencoderKL"], | ||
| } | ||
| session.get.return_value = response | ||
| repo_files = [ | ||
| "model_index.json", | ||
| "model.safetensors", | ||
| "unet/config.json", | ||
| "unet/diffusion_pytorch_model.safetensors", | ||
| "vae/config.json", | ||
| "vae/diffusion_pytorch_model.safetensors", | ||
| "README.md", | ||
| ] | ||
| metadata = HuggingFaceMetadata( | ||
| id="org/diffusers-repo", | ||
| name="diffusers-repo", | ||
| files=[ | ||
| RemoteModelFile( | ||
| url=f"https://huggingface.co/org/diffusers-repo/resolve/main/{path}", | ||
| path=Path("diffusers-repo", path), | ||
| ) | ||
| for path in repo_files | ||
| ], | ||
| ) | ||
|
|
||
| selected = {file.path.as_posix() for file in metadata.download_urls(session=session)} | ||
|
|
||
| assert selected == { | ||
| "diffusers-repo/model_index.json", | ||
| "diffusers-repo/unet/config.json", | ||
| "diffusers-repo/unet/diffusion_pytorch_model.safetensors", | ||
| "diffusers-repo/vae/config.json", | ||
| "diffusers-repo/vae/diffusion_pytorch_model.safetensors", | ||
| } | ||
| assert session.get.call_args.args[0].endswith("model_index.json") | ||
|
|
||
|
|
||
| def test_repo_id_diffusers_subfolder_uses_its_model_index_components() -> None: | ||
| session = Mock() | ||
| response = Mock(status_code=200) | ||
| response.json.return_value = { | ||
| "_class_name": "StableDiffusionPipeline", | ||
| "unet": ["diffusers", "UNet2DConditionModel"], | ||
| "vae": ["diffusers", "AutoencoderKL"], | ||
| } | ||
| session.get.return_value = response | ||
| repo_files = [ | ||
| "pipeline/model_index.json", | ||
| "pipeline/model.safetensors", | ||
| "pipeline/unet/config.json", | ||
| "pipeline/unet/diffusion_pytorch_model.safetensors", | ||
| "pipeline/vae/config.json", | ||
| "pipeline/vae/diffusion_pytorch_model.safetensors", | ||
| ] | ||
| metadata = HuggingFaceMetadata( | ||
| id="org/diffusers-repo", | ||
| name="diffusers-repo", | ||
| files=[ | ||
| RemoteModelFile( | ||
| url=f"https://huggingface.co/org/diffusers-repo/resolve/main/{path}", | ||
| path=Path("diffusers-repo", path), | ||
| ) | ||
| for path in repo_files | ||
| ], | ||
| ) | ||
|
|
||
| selected = {file.path.as_posix() for file in metadata.download_urls(subfolder=Path("pipeline"), session=session)} | ||
|
|
||
| assert selected == { | ||
| "diffusers-repo/pipeline/model_index.json", | ||
| "diffusers-repo/pipeline/unet/config.json", | ||
| "diffusers-repo/pipeline/unet/diffusion_pytorch_model.safetensors", | ||
| "diffusers-repo/pipeline/vae/config.json", | ||
| "diffusers-repo/pipeline/vae/diffusion_pytorch_model.safetensors", | ||
| } | ||
| assert session.get.call_args.args[0].endswith("pipeline/model_index.json") |
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.
nit: In production every path starts with the repo name, because
from_id()buildsPath(name, rfilename). So thePath()fallback only serves the older unprefixed tests.If the two ever disagree, the filter silently switches itself off again, which is exactly #9901. I'd use
Path(self.name)unconditionally and move those tests to prefixed paths.