ComfyUI icon indicating copy to clipboard operation
ComfyUI copied to clipboard

Update extra_config.py

Open dchatel opened this issue 10 months ago • 10 comments

added path normalization to full_path.

needed to ensure that line 204 in folder_paths.py: if full_folder_path in paths: works properly on all OS.

dchatel avatar Jan 12 '25 13:01 dchatel

needed to ensure that line 204 in folder_paths.py: if full_folder_path in paths: works properly on all OS.

In my opinion, a good start would be to modify existing tests or add a new test that explains why these changes are necessary.

bigcat88 avatar Jan 13 '25 10:01 bigcat88

An example of why this is at least desirable is that without this, if a user sets a custom base_path and is_default to true, then the model downloader in comfyui-manager does not work as intended.

You can try it yourself: set the base_path somewhere external to comfyui, set is_default to true, then try to download a model using comfyui-manager, or let a custom node download its own model (like comfyui-florence2). You'll see that the models get download inside the comfyui/model folder instead of the one you've set in base_path.

dchatel avatar Jan 13 '25 12:01 dchatel

Then this is not quite the correct implementation for this.

There can be many be several records with is_default for different folder paths, it is impossible to determine which of them should ultimately overwrite the root models_dir.

Currently software that performs download should look under the specific keys in the global folder_names_and_paths variable and get path to store models from it.

@comfyanonymous would you like to extend the functionality of the yaml file and allow it to set the models_dir variable?

bigcat88 avatar Jan 13 '25 12:01 bigcat88

In #5864, @vvuk suggested that base paths be processed in the order they were defined in extra_models_paths.yaml, with the first to be defined taking precedence.

iwr-redmond avatar Jan 17 '25 22:01 iwr-redmond

(I'd appreciate some feedback/guidance in #5864 -- I'm not quite sure what the dev process is here, let me know if I should be doing something else for PR submissions!)

vvuk avatar Jan 17 '25 22:01 vvuk

There appears to be a backlog of YAML file PRs:

#2720 by @soof-golan #3744 by @iwanders #4365 by @ltdrdata

iwr-redmond avatar Jan 17 '25 22:01 iwr-redmond

An example of why this is at least desirable is that without this, if a user sets a custom base_path and is_default to true, then the model downloader in comfyui-manager does not work as intended.

You can try it yourself: set the base_path somewhere external to comfyui, set is_default to true, then try to download a model using comfyui-manager, or let a custom node download its own model (like comfyui-florence2). You'll see that the models get download inside the comfyui/model folder instead of the one you've set in base_path.

The model download path used by ComfyUI-Manager is the one set as download_model_base in the path section where is_default is set to true.

This is not documented yet, but it will be added to the README.md soon.

https://github.com/ltdrdata/ComfyUI-Manager/blob/ddb3c4e3ce446a713724b68013cedb16b2049b1a/glob/manager_server.py#L250

ltdrdata avatar Jan 18 '25 09:01 ltdrdata

Ok, but this is still not the same behavior as the one suggested here. The idea is to add an option in the extra_model_paths to the models path, to allow users to specify a custom models path instead of comfyui/models.

For example, if I set the following in extra_models_paths:

comfyui:
    is_default: true
    download_model_base: g:/ai/models/

and run comfyui, comfyui can't find any models, even though it might download new models in that folder.

Maybe having a parameter models_path or something like that would be a nice addition. If you want to have multiple comfyui installations, for example, for testing, debugging, or development/PR purposes, having the possibility to use the same models path for every install is really interesting.

dchatel avatar Jan 22 '25 06:01 dchatel

Ok, but this is still not the same behavior as the one suggested here. The idea is to add an option in the extra_model_paths to the models path, to allow users to specify a custom models path instead of comfyui/models.

For example, if I set the following in extra_models_paths:

comfyui:
    is_default: true
    download_model_base: g:/ai/models/

and run comfyui, comfyui can't find any models, even though it might download new models in that folder.

Maybe having a parameter models_path or something like that would be a nice addition. If you want to have multiple comfyui installations, for example, for testing, debugging, or development/PR purposes, having the possibility to use the same models path for every install is really interesting.

download_model_base and other model settings are entirely different concepts. You can configure multiple model paths as you wish. However, there can only be one download path, and that is specified in download_model_base.

ltdrdata avatar Jan 22 '25 07:01 ltdrdata

Ok, but this is still not the same behavior as the one suggested here. The idea is to add an option in the extra_model_paths to the models path, to allow users to specify a custom models path instead of comfyui/models.

For example, if I set the following in extra_models_paths:

comfyui:
    is_default: true
    download_model_base: g:/ai/models/

and run comfyui, comfyui can't find any models, even though it might download new models in that folder.

Maybe having a parameter models_path or something like that would be a nice addition. If you want to have multiple comfyui installations, for example, for testing, debugging, or development/PR purposes, having the possibility to use the same models path for every install is really interesting.

Adding the startup parameter --base-directory path/ComfyUI_User_Data and move the folders custom_nodes, input, models, output, temp, user to the custom ComfyUI_User_Data folder is very helpful.

billwuhao avatar Apr 27 '25 05:04 billwuhao