Add ResolveImage and PublishImage nodes #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: David Feltell <[email protected]>
See https://github.com/Comfy-Org/cookiecutter-comfy-extension Signed-off-by: David Feltell <[email protected]>
Ensure all files that can support comments have an SPDX license identifier. Signed-off-by: David Feltell <[email protected]>
Set up ruff to enforce a 99 char line limit, mirroring other OpenAssetIO projects. Configure yml with 2-space tabs, mirroring other OpenAssetIO projects (and the ComfyUI cookiecutter project's defaults). Bulk reformat using PyCharm configured to use ruff - i.e. Python files are reformatted using ruff, whilst other files are reformatted according to .editorconfig and/or PyCharm defaults. Signed-off-by: David Feltell <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
feltech
force-pushed
the
work/resolve_and_publish_images
branch
from
6fe2744 to
6a5fb08
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds OpenAssetIO integration to ComfyUI by implementing ResolveImage and PublishImage nodes that allow workflows to resolve assets from and publish results to asset management systems. The implementation follows the ComfyUI custom node development pattern and includes comprehensive test coverage and project configuration based on the cookiecutter-comfy-extension template.
- Adds ResolveImage node to resolve entity references into image tensors for workflow processing
- Adds PublishImage node as an output node to publish images back to asset management systems
- Sets up complete project infrastructure including tests, linting, CI/CD, and documentation
Reviewed Changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/openassetio_comfyui/nodes.py | Core implementation of ResolveImage and PublishImage nodes with OpenAssetIO integration |
| src/openassetio_comfyui/init.py | Package initialization exposing node mappings for ComfyUI discovery |
| init.py | Top-level init for direct repository checkout under ComfyUI custom_nodes |
| tests/test_openassetio_comfyui.py | Comprehensive test suite covering node functionality and edge cases |
| tests/resources/ | Test configuration and mock data for OpenAssetIO BAL manager |
| pyproject.toml | Project configuration with dependencies, build settings, and linting rules |
| README.md | Documentation covering installation, usage, and development |
| .github/workflows/ | CI/CD workflows for testing, publishing, and validation |
| .pre-commit-config.yaml | Pre-commit hooks for code quality |
| .editorconfig | Editor configuration for consistent formatting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Copy-paste current implementation of core LoadImage, but use a string input providing an entity reference instead of a file upload, and retrieve input file path from the OpenAssetIO manager. Due to the mix-and-match by ComfyUI of class vs. instance methods, plus no way to inject context, use a singleton class to represent the OpenAssetIO host application. Signed-off-by: David Feltell <[email protected]>
Copy-paste current implementation of core SaveImage, but use a string input providing an entity reference instead of a file name, and retrieve target file path from the OpenAssetIO manager. Support image preview in the UI by copying the published file to a (temp) directory on ComfyUI's allow-list. Signed-off-by: David Feltell <[email protected]>
ComfyUI discovers plugins from a `custom_nodes` directory, and so it doesn't use the Python environment to locate plugins. However, we have dependencies that must be installed into the Python environment for our plugin to work. It is implied in the docs that bundling a `requirements.txt` file is how the ComfyUI CLI and Manager discovers and installs plugin dependencies. In addition, a separate `requirements.txt` allows cleaner manual installation (i.e. clone the repo under `ComfyUI/custom_nodes` then `pip install -r requirements.txt`) Signed-off-by: David Feltell <[email protected]>
Signed-off-by: David Feltell <[email protected]>
feltech
force-pushed
the
work/resolve_and_publish_images
branch
14 times, most recently
from
475e3c2 to
512dc8f
Compare
At the moment, we have no history to diff against, resulting in an error > Error loading nodes: Could not find __init__.py in base_repo This commit should be reverted once the initial node implementation has been merged to main. Signed-off-by: David Feltell <[email protected]>
feltech
force-pushed
the
work/resolve_and_publish_images
branch
from
512dc8f to
357c8c3
Compare
Pin the Python version to 3.11, since OpenAssetIO is currently unavailable on PyPI for 3.12 (i.e. only currently supports VFX Reference Platform CY23/24). Checkout ComfyUI and add to PYTHONPATH for tests, since we make use of its internal utility libraries. We also make use of some of the dependencies of ComfyUI (e.g. numpy), so install ComfyUI's dependencies into the environment. Signed-off-by: David Feltell <[email protected]>
feltech
force-pushed
the
work/resolve_and_publish_images
branch
from
357c8c3 to
e793b74
Compare
|
^ the above bunch of force-pushes are primarily trying to get the CI workflow to run. However, despite fiddling with various settings, the workflow will not run. I believe this is a security restriction, which I can't find a way to disable (but probably shouldn't disable anyway). Possibly to do with current To see the CI running successfully, I've mirrored this PR in my fork: feltech#2 I think we just have to merge this, and trust that the CI will run on subsequent PRs (we can test this immediately after merging). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add ResolveImage node to resolve an entity reference into an image tensor for processing by the workflow.
Add PublishImage node as an output node, which publishes an incoming image to an entity reference.
Use https://github.com/Comfy-Org/cookiecutter-comfy-extension as a starting point. Hence update the template project to add necessary supporting project configuration, including tests, linter config, CI config, and README doc.