mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-07-06 22:51:18 +08:00
feat(assets): drop logical_path from the asset response
Ratified in the loader-key sync: the namespaced locator buys nothing today (hash/ID-based locating is the long-term direction), and keeping it adds a third path-shaped field for clients to confuse. loader_path and display_name carry the loader and display concerns; the storage-root matching stays internal, still powering display_name. Response-surface only; no column or migration involved.
This commit is contained in:
parent
bd7d95bdb4
commit
529abc991f
@ -40,7 +40,7 @@ from app.assets.services import (
|
|||||||
)
|
)
|
||||||
from app.assets.services.cursor import InvalidCursorError
|
from app.assets.services.cursor import InvalidCursorError
|
||||||
from app.assets.services.path_utils import (
|
from app.assets.services.path_utils import (
|
||||||
compute_asset_response_paths,
|
compute_display_name,
|
||||||
compute_loader_path,
|
compute_loader_path,
|
||||||
)
|
)
|
||||||
from app.assets.services.tagging import list_tag_histogram
|
from app.assets.services.tagging import list_tag_histogram
|
||||||
@ -165,8 +165,7 @@ def _build_asset_response(result: schemas.AssetDetailResult | schemas.UploadResu
|
|||||||
else:
|
else:
|
||||||
preview_url = _build_preview_url_from_view(result.tags, result.ref.user_metadata)
|
preview_url = _build_preview_url_from_view(result.tags, result.ref.user_metadata)
|
||||||
if result.ref.file_path:
|
if result.ref.file_path:
|
||||||
paths = compute_asset_response_paths(result.ref.file_path)
|
display_name = compute_display_name(result.ref.file_path)
|
||||||
logical_path, display_name = paths if paths else (None, None)
|
|
||||||
# In-root loader path (model category dropped): what model loaders consume.
|
# In-root loader path (model category dropped): what model loaders consume.
|
||||||
# Persisted at scan/ingest; fall back to computing for rows written
|
# Persisted at scan/ingest; fall back to computing for rows written
|
||||||
# before the column existed.
|
# before the column existed.
|
||||||
@ -174,14 +173,13 @@ def _build_asset_response(result: schemas.AssetDetailResult | schemas.UploadResu
|
|||||||
if loader_path is None:
|
if loader_path is None:
|
||||||
loader_path = compute_loader_path(result.ref.file_path)
|
loader_path = compute_loader_path(result.ref.file_path)
|
||||||
else:
|
else:
|
||||||
logical_path, display_name, loader_path = None, None, None
|
display_name, loader_path = None, None
|
||||||
asset_content_hash = result.asset.hash if result.asset else None
|
asset_content_hash = result.asset.hash if result.asset else None
|
||||||
return schemas_out.Asset(
|
return schemas_out.Asset(
|
||||||
id=result.ref.id,
|
id=result.ref.id,
|
||||||
name=result.ref.name,
|
name=result.ref.name,
|
||||||
hash=asset_content_hash,
|
hash=asset_content_hash,
|
||||||
loader_path=loader_path,
|
loader_path=loader_path,
|
||||||
logical_path=logical_path,
|
|
||||||
display_name=display_name,
|
display_name=display_name,
|
||||||
asset_hash=asset_content_hash,
|
asset_hash=asset_content_hash,
|
||||||
size=int(result.asset.size_bytes) if result.asset else None,
|
size=int(result.asset.size_bytes) if result.asset else None,
|
||||||
|
|||||||
@ -12,20 +12,16 @@ class Asset(BaseModel):
|
|||||||
name: str = Field(
|
name: str = Field(
|
||||||
...,
|
...,
|
||||||
deprecated=True,
|
deprecated=True,
|
||||||
description="Reference label, often caller-provided or derived from the filename. Deprecated for storage path/display semantics; use `loader_path`, `logical_path`, and `display_name` when present.",
|
description="Reference label, often caller-provided or derived from the filename. Deprecated for storage path/display semantics; use `loader_path` and `display_name` when present.",
|
||||||
)
|
)
|
||||||
hash: str | None = None
|
hash: str | None = None
|
||||||
loader_path: str | None = Field(
|
loader_path: str | None = Field(
|
||||||
default=None,
|
default=None,
|
||||||
description="In-root loader path for filesystem-backed assets: the path relative to its storage root with the top-level model category dropped (e.g. `models/checkpoints/foo/bar.safetensors` -> `foo/bar.safetensors`). This is the value model loaders consume. `None` when the file is not within a recognized root or model category.",
|
description="In-root loader path for filesystem-backed assets: the path relative to its storage root with the top-level model category dropped (e.g. `models/checkpoints/foo/bar.safetensors` -> `foo/bar.safetensors`). This is the value model loaders consume. `None` when the file is not within a recognized root or model category.",
|
||||||
)
|
)
|
||||||
logical_path: str | None = Field(
|
|
||||||
default=None,
|
|
||||||
description="Runtime storage locator for filesystem-backed assets, using Comfy storage namespaces such as `input/`, `output/`, `temp/`, or `models/` (e.g. `models/checkpoints/foo/bar.safetensors`). Not an absolute filesystem path, unique identity, or model loader path.",
|
|
||||||
)
|
|
||||||
display_name: str | None = Field(
|
display_name: str | None = Field(
|
||||||
default=None,
|
default=None,
|
||||||
description="Human-facing label derived from `logical_path`, usually the path below the top-level storage namespace. Not unique.",
|
description="Human-facing label for filesystem-backed assets: the path below the top-level storage namespace (e.g. `checkpoints/foo/bar.safetensors` under `models/`). Not unique.",
|
||||||
)
|
)
|
||||||
asset_hash: str | None = None
|
asset_hash: str | None = None
|
||||||
size: int | None = None
|
size: int | None = None
|
||||||
|
|||||||
@ -84,11 +84,14 @@ def _is_relative_to(child: str, parent: str) -> bool:
|
|||||||
|
|
||||||
|
|
||||||
def compute_asset_response_paths(file_path: str) -> tuple[str, str | None] | None:
|
def compute_asset_response_paths(file_path: str) -> tuple[str, str | None] | None:
|
||||||
"""Return public (file_path, display_name) response fields for a file path.
|
"""Return (logical_path, display_name) for a file path.
|
||||||
|
|
||||||
These fields are storage locators, not model-loader namespaces. Registered
|
``logical_path`` is the internal namespaced storage locator (e.g.
|
||||||
model-folder membership is represented by backend tags such as
|
``models/checkpoints/foo/bar.safetensors``); ``display_name`` is the
|
||||||
``model_type:<folder_name>``; response paths only use known storage roots.
|
human-facing label below that namespace, served on Asset responses. These
|
||||||
|
are storage locators, not model-loader namespaces. Registered model-folder
|
||||||
|
membership is represented by backend tags such as
|
||||||
|
``model_type:<folder_name>``; these paths only use known storage roots.
|
||||||
"""
|
"""
|
||||||
fp_abs = os.path.abspath(file_path)
|
fp_abs = os.path.abspath(file_path)
|
||||||
candidates: list[tuple[int, int, str, str]] = []
|
candidates: list[tuple[int, int, str, str]] = []
|
||||||
@ -123,7 +126,7 @@ def compute_display_name(file_path: str) -> str | None:
|
|||||||
|
|
||||||
|
|
||||||
def compute_logical_path(file_path: str) -> str | None:
|
def compute_logical_path(file_path: str) -> str | None:
|
||||||
"""Return the asset's namespaced storage `logical_path`, or None for unknown paths."""
|
"""Return the internal namespaced storage locator, or None for unknown paths."""
|
||||||
result = compute_asset_response_paths(file_path)
|
result = compute_asset_response_paths(file_path)
|
||||||
return result[0] if result else None
|
return result[0] if result else None
|
||||||
|
|
||||||
@ -136,9 +139,9 @@ def compute_loader_path(file_path: str) -> str | None:
|
|||||||
/.../models/text_encoders/clip_g.safetensors -> "clip_g.safetensors"
|
/.../models/text_encoders/clip_g.safetensors -> "clip_g.safetensors"
|
||||||
|
|
||||||
This is the value model loaders consume (the model category is dropped). It
|
This is the value model loaders consume (the model category is dropped). It
|
||||||
backs the public Asset response `file_path` field and the internal
|
is persisted as ``AssetReference.loader_path`` and served as the public
|
||||||
``computed_filename`` metadata. The namespaced storage locator (`logical_path`)
|
Asset response `loader_path` field. The human-facing `display_name` comes
|
||||||
and human-facing `display_name` come from compute_asset_response_paths().
|
from compute_asset_response_paths().
|
||||||
|
|
||||||
For input/output/temp paths the full path relative to that root is returned.
|
For input/output/temp paths the full path relative to that root is returned.
|
||||||
For paths outside any known root, returns None.
|
For paths outside any known root, returns None.
|
||||||
|
|||||||
@ -15,12 +15,8 @@ components:
|
|||||||
description: 'In-root loader path for filesystem-backed assets: the path relative to its storage root with the top-level model category dropped (e.g. `models/checkpoints/foo/bar.safetensors` -> `foo/bar.safetensors`). This is the value model loaders consume. `None` when the file is not within a recognized root or model category.'
|
description: 'In-root loader path for filesystem-backed assets: the path relative to its storage root with the top-level model category dropped (e.g. `models/checkpoints/foo/bar.safetensors` -> `foo/bar.safetensors`). This is the value model loaders consume. `None` when the file is not within a recognized root or model category.'
|
||||||
nullable: true
|
nullable: true
|
||||||
type: string
|
type: string
|
||||||
logical_path:
|
|
||||||
description: Runtime storage locator for filesystem-backed assets, using Comfy storage namespaces such as `input/`, `output/`, `temp/`, or `models/` (e.g. `models/checkpoints/foo/bar.safetensors`). Not an absolute filesystem path, unique identity, or model loader path.
|
|
||||||
nullable: true
|
|
||||||
type: string
|
|
||||||
display_name:
|
display_name:
|
||||||
description: Human-facing label derived from `logical_path`, usually the path below the top-level storage namespace. Not unique.
|
description: 'Human-facing label for filesystem-backed assets: the path below the top-level storage namespace (e.g. `checkpoints/foo/bar.safetensors` under `models/`). Not unique.'
|
||||||
nullable: true
|
nullable: true
|
||||||
type: string
|
type: string
|
||||||
id:
|
id:
|
||||||
|
|||||||
@ -69,7 +69,6 @@ def test_falls_back_to_compute_when_stored_loader_path_is_null(tmp_path: Path):
|
|||||||
resp = _build_asset_response(result)
|
resp = _build_asset_response(result)
|
||||||
|
|
||||||
assert resp.loader_path == "bar.safetensors"
|
assert resp.loader_path == "bar.safetensors"
|
||||||
assert resp.logical_path == "models/checkpoints/bar.safetensors"
|
|
||||||
assert resp.display_name == "checkpoints/bar.safetensors"
|
assert resp.display_name == "checkpoints/bar.safetensors"
|
||||||
|
|
||||||
|
|
||||||
@ -80,5 +79,4 @@ def test_all_path_fields_null_without_file_path():
|
|||||||
resp = _build_asset_response(result)
|
resp = _build_asset_response(result)
|
||||||
|
|
||||||
assert resp.loader_path is None
|
assert resp.loader_path is None
|
||||||
assert resp.logical_path is None
|
|
||||||
assert resp.display_name is None
|
assert resp.display_name is None
|
||||||
|
|||||||
@ -233,15 +233,13 @@ def test_upload_multiple_tags_fields_are_merged(http: requests.Session, api_base
|
|||||||
(
|
(
|
||||||
"tags",
|
"tags",
|
||||||
"extension",
|
"extension",
|
||||||
"expected_prefix",
|
|
||||||
"expected_display_prefix",
|
"expected_display_prefix",
|
||||||
),
|
),
|
||||||
[
|
[
|
||||||
(["input", "unit-tests"], ".png", "input", ""),
|
(["input", "unit-tests"], ".png", ""),
|
||||||
(
|
(
|
||||||
["models", "model_type:checkpoints", "unit-tests"],
|
["models", "model_type:checkpoints", "unit-tests"],
|
||||||
".safetensors",
|
".safetensors",
|
||||||
"models/checkpoints",
|
|
||||||
"checkpoints/",
|
"checkpoints/",
|
||||||
),
|
),
|
||||||
],
|
],
|
||||||
@ -249,7 +247,6 @@ def test_upload_multiple_tags_fields_are_merged(http: requests.Session, api_base
|
|||||||
def test_upload_response_includes_loader_path_and_display_name(
|
def test_upload_response_includes_loader_path_and_display_name(
|
||||||
tags: list[str],
|
tags: list[str],
|
||||||
extension: str,
|
extension: str,
|
||||||
expected_prefix: str,
|
|
||||||
expected_display_prefix: str,
|
expected_display_prefix: str,
|
||||||
http: requests.Session,
|
http: requests.Session,
|
||||||
api_base: str,
|
api_base: str,
|
||||||
@ -270,20 +267,18 @@ def test_upload_response_includes_loader_path_and_display_name(
|
|||||||
assert created_r.status_code in (200, 201), created
|
assert created_r.status_code in (200, 201), created
|
||||||
stored_filename = get_asset_filename(created["asset_hash"], extension)
|
stored_filename = get_asset_filename(created["asset_hash"], extension)
|
||||||
expected_suffix = stored_filename
|
expected_suffix = stored_filename
|
||||||
expected_logical_path = f"{expected_prefix}/{expected_suffix}"
|
|
||||||
expected_display_name = f"{expected_display_prefix}{expected_suffix}"
|
expected_display_name = f"{expected_display_prefix}{expected_suffix}"
|
||||||
# In-root loader path: model category dropped, no subfolders here -> just the filename.
|
# In-root loader path: model category dropped, no subfolders here -> just the filename.
|
||||||
expected_loader_path = expected_suffix
|
expected_loader_path = expected_suffix
|
||||||
|
|
||||||
assert created["loader_path"] == expected_loader_path
|
assert created["loader_path"] == expected_loader_path
|
||||||
assert created["logical_path"] == expected_logical_path
|
|
||||||
assert created["display_name"] == expected_display_name
|
assert created["display_name"] == expected_display_name
|
||||||
|
assert "logical_path" not in created
|
||||||
|
|
||||||
detail_r = http.get(f"{api_base}/api/assets/{created['id']}", timeout=120)
|
detail_r = http.get(f"{api_base}/api/assets/{created['id']}", timeout=120)
|
||||||
detail = detail_r.json()
|
detail = detail_r.json()
|
||||||
assert detail_r.status_code == 200, detail
|
assert detail_r.status_code == 200, detail
|
||||||
assert detail["loader_path"] == expected_loader_path
|
assert detail["loader_path"] == expected_loader_path
|
||||||
assert detail["logical_path"] == expected_logical_path
|
|
||||||
assert detail["display_name"] == expected_display_name
|
assert detail["display_name"] == expected_display_name
|
||||||
|
|
||||||
list_r = http.get(
|
list_r = http.get(
|
||||||
@ -295,7 +290,6 @@ def test_upload_response_includes_loader_path_and_display_name(
|
|||||||
assert list_r.status_code == 200, listed
|
assert list_r.status_code == 200, listed
|
||||||
match = next(a for a in listed["assets"] if a["id"] == created["id"])
|
match = next(a for a in listed["assets"] if a["id"] == created["id"])
|
||||||
assert match["loader_path"] == expected_loader_path
|
assert match["loader_path"] == expected_loader_path
|
||||||
assert match["logical_path"] == expected_logical_path
|
|
||||||
assert match["display_name"] == expected_display_name
|
assert match["display_name"] == expected_display_name
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user