mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-05-27 17:37:39 +08:00
Use module_parent string directly as 'source'; drop fixed-enum mapping
The public 'source' field on each NODE_STARTUP_ERRORS entry is now the same string as the internal module_parent passed to load_custom_node ('custom_nodes', 'comfy_extras', 'comfy_api_nodes'), rather than being translated to a separate fixed enum. Treating it as a free-form string keeps the contract durable in case the node-source layout evolves (e.g. comfy_api_nodes eventually moving out of core).
The API endpoint now also dynamically groups by whatever sources are present rather than hardcoding the three known top-level keys; consumers should not assume any particular set of keys is always present.
Drops the _NODE_SOURCE_BY_PARENT map, _node_source_from_parent helper, and the related test. Adds a test covering an arbitrary unknown module_parent value passing through unchanged.
Ref: ComfyUI-Launcher#303
Amp-Thread-ID: https://ampcode.com/threads/T-019e23a1-2acc-7619-bd0e-f783d1368ef3
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
parent
ba1c039a04
commit
07ff14ae02
2
main.py
2
main.py
@ -144,7 +144,7 @@ def execute_prestartup_script():
|
||||
from nodes import record_node_startup_error
|
||||
record_node_startup_error(
|
||||
module_path=os.path.dirname(script_path),
|
||||
source="custom_node",
|
||||
source="custom_nodes",
|
||||
phase="prestartup",
|
||||
error=e,
|
||||
tb=traceback.format_exc(),
|
||||
|
||||
25
nodes.py
25
nodes.py
@ -2181,24 +2181,17 @@ EXTENSION_WEB_DIRS = {}
|
||||
# Dictionary of successfully loaded module names and associated directories.
|
||||
LOADED_MODULE_DIRS = {}
|
||||
|
||||
# Mapping from internal module_parent values to the public "source"
|
||||
# category the API exposes. Keeps the on-disk layout decoupled from
|
||||
# the names the frontend/Manager switches on.
|
||||
_NODE_SOURCE_BY_PARENT = {
|
||||
"custom_nodes": "custom_node",
|
||||
"comfy_extras": "comfy_extra",
|
||||
"comfy_api_nodes": "api_node",
|
||||
}
|
||||
|
||||
|
||||
def _node_source_from_parent(module_parent: str) -> str:
|
||||
return _NODE_SOURCE_BY_PARENT.get(module_parent, "custom_node")
|
||||
|
||||
|
||||
# Dictionary of custom node startup errors, keyed by "<source>:<module_name>"
|
||||
# so that name collisions across custom_nodes / comfy_extras / comfy_api_nodes
|
||||
# do not overwrite each other. Each value contains: source, module_name,
|
||||
# module_path, error, traceback, phase.
|
||||
#
|
||||
# `source` is the same string as the internal `module_parent` used at load
|
||||
# time (e.g. "custom_nodes", "comfy_extras", "comfy_api_nodes"). It is
|
||||
# intentionally a free-form string rather than a fixed enum so the contract
|
||||
# survives node-source layouts evolving (e.g. comfy_api_nodes eventually
|
||||
# moving out of core). Consumers should treat any new value as a new bucket
|
||||
# rather than rejecting it.
|
||||
NODE_STARTUP_ERRORS: dict[str, dict] = {}
|
||||
|
||||
|
||||
@ -2366,7 +2359,7 @@ async def load_custom_node(module_path: str, ignore=set(), module_parent="custom
|
||||
logging.warning(f"Error while calling comfy_entrypoint in {module_path}: {e}")
|
||||
record_node_startup_error(
|
||||
module_path=module_path,
|
||||
source=_node_source_from_parent(module_parent),
|
||||
source=module_parent,
|
||||
phase="entrypoint",
|
||||
error=e,
|
||||
tb=tb,
|
||||
@ -2381,7 +2374,7 @@ async def load_custom_node(module_path: str, ignore=set(), module_parent="custom
|
||||
logging.warning(f"Cannot import {module_path} module for custom nodes: {e}")
|
||||
record_node_startup_error(
|
||||
module_path=module_path,
|
||||
source=_node_source_from_parent(module_parent),
|
||||
source=module_parent,
|
||||
phase="import",
|
||||
error=e,
|
||||
tb=tb,
|
||||
|
||||
19
server.py
19
server.py
@ -755,17 +755,20 @@ class PromptServer():
|
||||
|
||||
@routes.get("/node_startup_errors")
|
||||
async def get_node_startup_errors(request):
|
||||
# Group errors by source ("custom_node" / "comfy_extra" / "api_node")
|
||||
# so the frontend/Manager can render them in distinct sections.
|
||||
# Group errors by source so the frontend/Manager can render them
|
||||
# in distinct sections. `source` is the same string as the
|
||||
# module_parent used at load time (e.g. "custom_nodes",
|
||||
# "comfy_extras", "comfy_api_nodes") and is left as a free-form
|
||||
# string so the contract survives node-source layouts evolving.
|
||||
# The response only contains source buckets that actually had a
|
||||
# failure; consumers should not assume any particular set of keys
|
||||
# is always present.
|
||||
#
|
||||
# `module_path` is stripped because the absolute on-disk path is
|
||||
# internal detail that the frontend has no use for.
|
||||
grouped: dict[str, dict[str, dict]] = {
|
||||
"custom_node": {},
|
||||
"comfy_extra": {},
|
||||
"api_node": {},
|
||||
}
|
||||
grouped: dict[str, dict[str, dict]] = {}
|
||||
for entry in nodes.NODE_STARTUP_ERRORS.values():
|
||||
source = entry.get("source", "custom_node")
|
||||
source = entry.get("source", "custom_nodes")
|
||||
public_entry = {k: v for k, v in entry.items() if k != "module_path"}
|
||||
grouped.setdefault(source, {})[entry["module_name"]] = public_entry
|
||||
return web.json_response(grouped)
|
||||
|
||||
@ -7,6 +7,7 @@ Covers:
|
||||
- Composite keying prevents collisions between modules with the same name
|
||||
in different sources.
|
||||
- record_node_startup_error stores the expected fields.
|
||||
- pyproject.toml metadata is attached when present and omitted when absent.
|
||||
"""
|
||||
import textwrap
|
||||
|
||||
@ -35,14 +36,14 @@ def test_record_node_startup_error_fields(tmp_path):
|
||||
err = ValueError("kaboom")
|
||||
nodes.record_node_startup_error(
|
||||
module_path=str(tmp_path / "my_pack"),
|
||||
source="custom_node",
|
||||
source="custom_nodes",
|
||||
phase="import",
|
||||
error=err,
|
||||
tb="traceback-text",
|
||||
)
|
||||
assert "custom_node:my_pack" in nodes.NODE_STARTUP_ERRORS
|
||||
entry = nodes.NODE_STARTUP_ERRORS["custom_node:my_pack"]
|
||||
assert entry["source"] == "custom_node"
|
||||
assert "custom_nodes:my_pack" in nodes.NODE_STARTUP_ERRORS
|
||||
entry = nodes.NODE_STARTUP_ERRORS["custom_nodes:my_pack"]
|
||||
assert entry["source"] == "custom_nodes"
|
||||
assert entry["module_name"] == "my_pack"
|
||||
assert entry["phase"] == "import"
|
||||
assert entry["error"] == "kaboom"
|
||||
@ -52,23 +53,20 @@ def test_record_node_startup_error_fields(tmp_path):
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"module_parent,expected_source",
|
||||
[
|
||||
("custom_nodes", "custom_node"),
|
||||
("comfy_extras", "comfy_extra"),
|
||||
("comfy_api_nodes", "api_node"),
|
||||
],
|
||||
"module_parent",
|
||||
["custom_nodes", "comfy_extras", "comfy_api_nodes"],
|
||||
)
|
||||
async def test_load_custom_node_records_source(tmp_path, module_parent, expected_source):
|
||||
async def test_load_custom_node_records_source(tmp_path, module_parent):
|
||||
# `source` in the entry should be the same string as `module_parent`.
|
||||
module_path = _write_broken_module(tmp_path, "broken_pack")
|
||||
|
||||
success = await nodes.load_custom_node(module_path, module_parent=module_parent)
|
||||
assert success is False
|
||||
|
||||
key = f"{expected_source}:broken_pack"
|
||||
key = f"{module_parent}:broken_pack"
|
||||
assert key in nodes.NODE_STARTUP_ERRORS, nodes.NODE_STARTUP_ERRORS
|
||||
entry = nodes.NODE_STARTUP_ERRORS[key]
|
||||
assert entry["source"] == expected_source
|
||||
assert entry["source"] == module_parent
|
||||
assert entry["module_name"] == "broken_pack"
|
||||
assert entry["phase"] == "import"
|
||||
assert "boom from" in entry["error"]
|
||||
@ -77,7 +75,7 @@ async def test_load_custom_node_records_source(tmp_path, module_parent, expected
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_load_custom_node_collision_across_sources(tmp_path):
|
||||
# Same module name registered as both a custom_node and a comfy_extra;
|
||||
# Same module name registered as both a custom node and a comfy_extra;
|
||||
# composite keying should keep both entries.
|
||||
cn_dir = tmp_path / "cn"
|
||||
extras_dir = tmp_path / "extras"
|
||||
@ -89,11 +87,11 @@ async def test_load_custom_node_collision_across_sources(tmp_path):
|
||||
assert await nodes.load_custom_node(cn_path, module_parent="custom_nodes") is False
|
||||
assert await nodes.load_custom_node(extras_path, module_parent="comfy_extras") is False
|
||||
|
||||
assert "custom_node:nodes_audio" in nodes.NODE_STARTUP_ERRORS
|
||||
assert "comfy_extra:nodes_audio" in nodes.NODE_STARTUP_ERRORS
|
||||
assert "custom_nodes:nodes_audio" in nodes.NODE_STARTUP_ERRORS
|
||||
assert "comfy_extras:nodes_audio" in nodes.NODE_STARTUP_ERRORS
|
||||
assert (
|
||||
nodes.NODE_STARTUP_ERRORS["custom_node:nodes_audio"]["module_path"]
|
||||
!= nodes.NODE_STARTUP_ERRORS["comfy_extra:nodes_audio"]["module_path"]
|
||||
nodes.NODE_STARTUP_ERRORS["custom_nodes:nodes_audio"]["module_path"]
|
||||
!= nodes.NODE_STARTUP_ERRORS["comfy_extras:nodes_audio"]["module_path"]
|
||||
)
|
||||
|
||||
|
||||
@ -118,7 +116,7 @@ async def test_load_custom_node_attaches_pyproject_metadata(tmp_path):
|
||||
success = await nodes.load_custom_node(str(pack_dir), module_parent="custom_nodes")
|
||||
assert success is False
|
||||
|
||||
entry = nodes.NODE_STARTUP_ERRORS["custom_node:MyCoolPack"]
|
||||
entry = nodes.NODE_STARTUP_ERRORS["custom_nodes:MyCoolPack"]
|
||||
assert "pyproject" in entry, entry
|
||||
py = entry["pyproject"]
|
||||
assert py["pack_id"] == "comfyui-mycoolpack"
|
||||
@ -134,12 +132,15 @@ async def test_load_custom_node_no_pyproject_skips_metadata(tmp_path):
|
||||
# so the entry must not contain a 'pyproject' key.
|
||||
module_path = _write_broken_module(tmp_path, "lonely")
|
||||
assert await nodes.load_custom_node(module_path, module_parent="comfy_extras") is False
|
||||
entry = nodes.NODE_STARTUP_ERRORS["comfy_extra:lonely"]
|
||||
entry = nodes.NODE_STARTUP_ERRORS["comfy_extras:lonely"]
|
||||
assert "pyproject" not in entry
|
||||
|
||||
|
||||
def test_unknown_module_parent_defaults_to_custom_node():
|
||||
assert nodes._node_source_from_parent("custom_nodes") == "custom_node"
|
||||
assert nodes._node_source_from_parent("comfy_extras") == "comfy_extra"
|
||||
assert nodes._node_source_from_parent("comfy_api_nodes") == "api_node"
|
||||
assert nodes._node_source_from_parent("something_else") == "custom_node"
|
||||
@pytest.mark.asyncio
|
||||
async def test_load_custom_node_arbitrary_module_parent_passes_through(tmp_path):
|
||||
# `source` is a free-form string — an unknown module_parent (e.g. a future
|
||||
# node-source bucket) should be recorded as-is, not coerced or rejected.
|
||||
module_path = _write_broken_module(tmp_path, "future_pack")
|
||||
assert await nodes.load_custom_node(module_path, module_parent="future_source") is False
|
||||
entry = nodes.NODE_STARTUP_ERRORS["future_source:future_pack"]
|
||||
assert entry["source"] == "future_source"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user