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

Add support for custom load order of external nodes #4782

Closed

Conversation

DrakoTrogdor
Copy link

@DrakoTrogdor DrakoTrogdor commented Sep 4, 2024

Pull Request: Add Custom Module Load Order for External Nodes

Summary

This pull request introduces a new feature to support custom load ordering for external custom nodes. It adds the ability to read a module load order from a text file (custom_load_order.txt) and load custom nodes based on that order. This feature enhances the flexibility of module imports by allowing users to define the order in which custom nodes are loaded.

Changes Introduced

  1. Introduction of module_order_path:

    • A new variable module_order_path has been added, which points to a file (custom_nodes/custom_load_order.txt). This file contains a list of custom modules to be loaded in a specific order. Each line in the file represents the name of a module.
    +    module_order_path = 'custom_nodes/custom_load_order.txt'
  2. Reading the Custom Load Order File:

    • The code checks if the file exists at module_order_path. If it does, it reads the contents, stripping any excess whitespace, and stores each line (module name) in the list ordered_modules. If the file is not found, an empty list (ordered_modules) is initialized, meaning no specific order is applied.
    +    if os.path.exists(module_order_path):
    +        with open(module_order_path, 'r') as file:
    +            ordered_modules = [line.strip() for line in file]
    +    else:
    +        ordered_modules = []
  3. Creating order_map for Sorting:

    • A dictionary order_map is created to map module names to their index in the ordered_modules list. This mapping allows for efficient sorting of modules during the loading process.
    • If a module is not listed in custom_load_order.txt, it is assigned a default value of float('inf'), which ensures it will be loaded after the explicitly ordered modules.
    +    order_map = {name: i for i, name in enumerate(ordered_modules)}
  4. Sorting Custom Modules by Defined Order:

    • The list of custom modules (possible_modules) is sorted according to the indices defined in order_map. Modules that are listed in the order file are loaded first, and any modules not listed are loaded afterwards.
    +        possible_modules.sort(key=lambda x: order_map.get(x, float('inf')))

Reason for the Changes

These changes address the need to control the loading order of external custom nodes in the application. Previously, there was no mechanism to enforce a specific load order, which could lead to unpredictable behavior when certain modules conflict with others. By introducing an optional load order via a configurable text file, users can ensure that modules are loaded in a particular sequence if desired.

Impact

  • Flexibility: Users can now define a specific load order for custom nodes, improving the customization and stability of node loading.
  • Backward Compatibility: If no custom load order file is provided, the application defaults to loading modules in an arbitrary order as before, ensuring backward compatibility.

Example Usage

To use this feature, create a file at custom_nodes/custom_load_order.txt with the following content, where each line is the name of a custom module:

moduleA
moduleB
moduleC

Modules will be loaded in the order specified in this file. Any module not included will be loaded afterward, in no particular order.

Testing

  • Tested the changes with and without the custom_load_order.txt file.
  • Verified that modules load in the order specified when the file is present.
  • Ensured that the system falls back to default behavior when the file is missing or empty.

Future Enhancements

  • The re module could be leveraged to add pattern-based matching for more advanced load ordering in the future, such as wildcard-based module selection.

@ltdrdata
Copy link
Collaborator

ltdrdata commented Sep 4, 2024

The issue of import order is actually extremely sensitive. While it can be useful for debugging purposes, guaranteeing a specific import order may incentivize custom node implementations to rely on that order.

Even if dependencies exist between custom nodes, the structure should be such that there's no need to guarantee a specific order.

In other words, a flexible structure where nodes are used on-demand at the point of execution is preferable.
Otherwise, we might completely eliminate the possibility of mutual dependencies.

@DrakoTrogdor
Copy link
Author

I used this to fix having multiple nodes that conflict, without it I am unable to load some workflows without renaming the git repositories to change the loading order, which then causes the custom node manager to "lose" knowledge of those nodes. I made sure to make this code as unobtrusive as possible so that it doesn't change anything except giving the end user more options that don't cause other (technical) issues. Custom node authors already have the ability to affect load order by changing the name of their nodes... 000_a1_comfyui-custom_node would almost always load first, where ZZZ-COMFYUI-Zartans-Node would probably be last. With this the end user is given more options that they don't have to use (but can). But from the sounds of your comment, I'm guessing there has been some debate about this already.

@ltdrdata
Copy link
Collaborator

ltdrdata commented Sep 4, 2024

I used this to fix having multiple nodes that conflict, without it I am unable to load some workflows without renaming the git repositories to change the loading order, which then causes the custom node manager to "lose" knowledge of those nodes. I made sure to make this code as unobtrusive as possible so that it doesn't change anything except giving the end user more options that don't cause other (technical) issues. Custom node authors already have the ability to affect load order by changing the name of their nodes... 000_a1_comfyui-custom_node would almost always load first, where ZZZ-COMFYUI-Zartans-Node would probably be last. With this the end user is given more options that they don't have to use (but can). But from the sounds of your comment, I'm guessing there has been some debate about this already.

The nodes are not imported based on their names, but rather in the order that the file system provides them. This order might sometimes correspond to alphabetical order, but it could also be based on installation order, or simply in whatever order the operating system decides to provide them. Therefore, you should not assume that the current import system follows any specific order.

    for custom_node_path in node_paths:
        possible_modules = os.listdir(os.path.realpath(custom_node_path))
        if "__pycache__" in possible_modules:
            possible_modules.remove("__pycache__")

        for possible_module in possible_modules:
            module_path = os.path.join(custom_node_path, possible_module)
            if os.path.isfile(module_path) and os.path.splitext(module_path)[1] != ".py": continue
            if module_path.endswith(".disabled"): continue
            time_before = time.perf_counter()
            success = load_custom_node(module_path, base_node_names, module_parent="custom_nodes")

Personally, I think the order functionality can be useful for purposes such as reproducing specific situations, and it can be used as an emergency measure for certain corner case issues. (Especially, it could serve as a self-help solution for nodes that are no longer maintained.)

The problem is that we shouldn't lead custom node developers to assume that they can guarantee a specific order.

@DrakoTrogdor
Copy link
Author

putting aside the differences between OS's and differing setups, I feel this change gives end users more agency over the way things work on their own system. If the custom node developers are forcing themselves into a specific order by using this custom_node_ordering.txt, they could probably force themselves into nodes.py anyway and force their own order.

I will leave this conversation as is, I don't want to step on any toes. If this PR isn't approved, then it will just be my own little helper (I am becoming more powerful than any Jedi has ever dreamed of) :)

@DrakoTrogdor DrakoTrogdor force-pushed the custom_nodes_ordering branch 5 times, most recently from 45029ba to e816e78 Compare September 11, 2024 16:29
@mcmonkey4eva mcmonkey4eva added User Support A user needs help with something, probably not a bug. and removed User Support A user needs help with something, probably not a bug. labels Sep 12, 2024
… based on custom_load_order.txt if available.\n- Modules not listed are loaded after those specified.
@DrakoTrogdor
Copy link
Author

Closing as it looks like it is not desired

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