pyisolate-support release refresh#13380
pyisolate-support release refresh#13380pollockjj merged 5 commits intoComfy-Org:pyisolate-supportfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Refreshes the repo’s pyisolate-support integration to align with the latest validated “clean release” branch, updating isolation plumbing (routing + manifest options), bumping the pyisolate dependency, and removing deprecated sealed-worker 3D output types/nodes.
Changes:
- Bump
pyisolateto0.10.2and propagate new isolation manifest config fields (share_torch_no_deps,extra_index_urls). - Rework isolated PromptServer route registration to buffer in-child and flush to host, including bridging legacy module-level
ROUTES. - Remove deprecated SavePLY/SaveNPZ nodes and sealed-worker
PLY/NPZtypes; add/adjust isolation tests for new behaviors.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/isolation/test_inner_model_state_dict.py | Adds coverage that isolated inner model exposes state_dict() for LoRA key discovery. |
| tests/isolation/test_cuda_wheels_and_env_flags.py | Extends manifest plumbing tests for new isolation config fields and route buffering behavior. |
| tests/isolation/singleton_boundary_helpers.py | Updates exact-relay helper to reset/flush buffered child routes during capture. |
| requirements.txt | Bumps pyisolate pin to 0.10.2. |
| nodes.py | Stops loading removed extra-node modules (nodes_save_ply.py, nodes_save_npz.py). |
| comfy/isolation/proxies/prompt_server_impl.py | Buffers route registrations in child and introduces flush-to-host; adds app stub for compatibility. |
| comfy/isolation/proxies/folder_paths_proxy.py | Removes verbose diagnostic logging from get_filename_list. |
| comfy/isolation/model_patcher_proxy.py | Exposes state_dict on _InnerModelProxy by delegating to model_state_dict(). |
| comfy/isolation/host_policy.py | Exports sandbox mode to PYISOLATE_SANDBOX_MODE env var when loading host policy. |
| comfy/isolation/extension_wrapper.py | Bridges legacy ROUTES declarations and adds flush_pending_routes() for host-triggered flush. |
| comfy/isolation/extension_loader.py | Plumbs new manifest config keys and triggers route flushing on cache-hit and cache-miss discovery paths. |
| comfy/isolation/child_hooks.py | Removes DIAG logs; adjusts error message prefix. |
| comfy/isolation/adapter.py | Updates PromptServer wiring to use PromptServerService/PromptServerStub; removes old numpy serializer block. |
| comfy/isolation/init.py | Removes DIAG logs from proxy initialization. |
| comfy_extras/nodes_save_ply.py | Removes deprecated PLY output node. |
| comfy_extras/nodes_save_npz.py | Removes deprecated NPZ output node. |
| comfy_api_sealed_worker/ply_types.py | Removes sealed-worker PLY type. |
| comfy_api_sealed_worker/npz_types.py | Removes sealed-worker NPZ type. |
| comfy_api_sealed_worker/init.py | Updates exports/docstring to reflect removed PLY/NPZ types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| raise ExtensionLoadError( | ||
| "[tool.comfy.isolation.share_torch_no_deps] must be a list of non-empty strings" | ||
| ) | ||
| extension_config["share_torch_no_deps"] = share_torch_no_deps |
There was a problem hiding this comment.
The validation checks dep.strip() but then stores the original share_torch_no_deps values unchanged. This can leave leading/trailing whitespace in dependency names (e.g., " timm "), which may later fail matching/installation. Consider normalizing by stripping each entry (and optionally de-duplicating) before writing to extension_config.
| extension_config["share_torch_no_deps"] = share_torch_no_deps | |
| extension_config["share_torch_no_deps"] = [dep.strip() for dep in share_torch_no_deps] |
| ): | ||
| raise ExtensionLoadError( | ||
| "[tool.comfy.isolation.extra_index_urls] must be a list of non-empty strings" | ||
| ) |
There was a problem hiding this comment.
Similar to share_torch_no_deps, the validation checks u.strip() but stores the original extra_index_urls strings unchanged. To avoid subtle issues from whitespace-only differences, normalize (strip) entries (and optionally de-duplicate) before adding them to extension_config.
| ) | |
| ) | |
| extra_index_urls = list(dict.fromkeys(u.strip() for u in extra_index_urls)) |
| for method, path, handler in cls._pending_child_routes: | ||
| try: | ||
| await cls._rpc.register_route_rpc(method, path, handler) | ||
| flushed += 1 | ||
| except Exception as e: | ||
| logger.error("%s Child route flush failed %s %s: %s", LOG_PREFIX, method, path, e) | ||
| cls._pending_child_routes = [] |
There was a problem hiding this comment.
flush_child_routes() clears _pending_child_routes even if one or more registrations fail (exceptions are logged but the route is dropped). That can permanently lose route registrations in transient failure scenarios. Consider only removing entries that were successfully flushed, or retaining failed entries for retry and returning counts for (flushed, failed).
| for method, path, handler in cls._pending_child_routes: | |
| try: | |
| await cls._rpc.register_route_rpc(method, path, handler) | |
| flushed += 1 | |
| except Exception as e: | |
| logger.error("%s Child route flush failed %s %s: %s", LOG_PREFIX, method, path, e) | |
| cls._pending_child_routes = [] | |
| failed_routes = [] | |
| for method, path, handler in cls._pending_child_routes: | |
| try: | |
| await cls._rpc.register_route_rpc(method, path, handler) | |
| flushed += 1 | |
| except Exception as e: | |
| logger.error("%s Child route flush failed %s %s: %s", LOG_PREFIX, method, path, e) | |
| failed_routes.append((method, path, handler)) | |
| cls._pending_child_routes = failed_routes |
Refresh pyisolate-support from the validated clean release branch.
This PR is the upstream submission of the same clean branch that already passed the local merge validation, clean-branch gate, and fork trial PR sweep.
Validation chain:
Branch head: 59937d0