Skip to content

Conversation

@benceruleanlu
Copy link
Member

@benceruleanlu benceruleanlu commented May 18, 2025

This PR adds a route to serve static web assets from the comfyui-embedded-docs python package.

Related Frontend PR: Comfy-Org/ComfyUI_frontend#3922
Related Docs PR: Comfy-Org/docs#120

Include HELP in node_info

Include HELP in ComfyNodeABC

Readd image to BFL Ultra

Move images to backend
@Amorano
Copy link
Contributor

Amorano commented May 19, 2025

The HELP variable should also accept URL endpoints to dynamically load HTML/MD so that nothing needs to be hard-coded (help info).

@benceruleanlu benceruleanlu marked this pull request as draft May 19, 2025 03:16
@benceruleanlu benceruleanlu marked this pull request as ready for review May 19, 2025 07:50
@benceruleanlu benceruleanlu marked this pull request as draft May 20, 2025 03:01
@Kosinkadink
Copy link
Member

Looks good on the backend PR 👍 Not sure if we should make the logging message be in a similar format as the other 'missing packages' ones, but I see why you have gone with the approach you did since technically missing the docs dep is not critical.

@benceruleanlu
Copy link
Member Author

BTW, before merge, version would have to be bumped to whatever is latest, even 0.1.0 is out of date RN.

Copy link
Member

@Kosinkadink Kosinkadink left a comment

Choose a reason for hiding this comment

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

Looks good, we'll just need to sync release with frontend PR once the conflicts are resolved: Comfy-Org/ComfyUI_frontend#3922

Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

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

@Kosinkadink It looks like this can be merged ahead of time - do we need to sync frontend release?

Comment on lines +217 to +219
except ImportError:
logging.info("comfyui-embedded-docs package not found")
return None
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this have a catch-all that logs and returns? An unhandled exception type here would terminate app start.

Copy link
Member

@Kosinkadink Kosinkadink Jun 1, 2025

Choose a reason for hiding this comment

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

This should be fine - it matches the other import checks in frontend_management; our code checks the return value of this func so it shouldn't be an issue. Assuming the frontend PR handles docs static path not existing, of course:
image

Copy link
Member

Choose a reason for hiding this comment

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

And yep, we can definitely merge it head of time.

@Kosinkadink Kosinkadink added Core Core team dependency Core-Important labels Jun 1, 2025
@comfyanonymous comfyanonymous merged commit 180db67 into Comfy-Org:master Jun 1, 2025
9 checks passed
adlerfaulkner pushed a commit to LucaLabsInc/ComfyUI that referenced this pull request Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Core Core team dependency Core-Important

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants