Skip to content
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

[DRAFT] Support new ComfyUI-Manager #139

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

[DRAFT] Support new ComfyUI-Manager #139

wants to merge 9 commits into from

Conversation

ltdrdata
Copy link
Member

requires ComfyUI-Manager/feat/cnr

  • fixed: install
  • added: show-versions

@james03160927
Copy link
Collaborator

Would have to update test_node and e2e test when it's ready for merge.

hidden=True,
)
@tracking.track_command("node")
def registry_install(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

registry_install -> we would need to update the commands inside the registry_web repository https://github.com/Comfy-Org/registry-web/blob/23948cfe4faa73f640a48139479a718a2d561ccd/components/nodes/NodeDetails.tsx#L209.

This should be replaced with a new command. node install <node_id>@<version>

)
@tracking.track_command("node")
def show_versions(
node_name: str,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node_name or node_id? Should we use node_id here?

# To check more robustly, verify up to the `.git` path.
manager_path = os.path.join(
self.workspace_path, "custom_nodes", "ComfyUI-Manager"
cached_manager_path = self.config_manager.get(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, we knew the absolute path of the comfyUI manager. With the new changes, the comfyUI manager directory name would change depending on which version you've installed (e.g., nightly vs. specific version). We need to figure out the directory of the ComfyUI Manager.

Let's add comments explaining what this logic is.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic may not work if we are caching the repository name for multiple workspace.

@@ -214,7 +214,9 @@ def execute(
if skip_manager:
print("Skipping installation of ComfyUI-Manager. (by --skip-manager)")
else:
manager_repo_dir = os.path.join(repo_dir, "custom_nodes", "ComfyUI-Manager")
manager_repo_dir = os.path.join(
repo_dir, "custom_nodes", "comfyui-manager@nightly"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need to change this. Depending on the version of comfyUI manager, the repo directory would change.

@telamonian telamonian force-pushed the main branch 3 times, most recently from a582b27 to 0348aa4 Compare August 27, 2024 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants