Skip to content

Allow changing folder_paths.base_path via command line argument. #6600

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
Jan 29, 2025
Merged

Allow changing folder_paths.base_path via command line argument. #6600

merged 19 commits into from
Jan 29, 2025

Conversation

webfiltered
Copy link
Collaborator

@webfiltered webfiltered commented Jan 25, 2025 •
edited
Loading

Current

  • The base_path and models_dir properties of folder_paths are set during file import.
  • Many extensions are using these directly.
  • The ComfyUI directory cannot be read-only.

Proposed (Updated w/requested refactor)

  • Add --base-directory command line argument.
  • Almost* no change to existing behaviour
      • cli_args is now imported prior to folder_paths - nil impact expected.

@webfiltered webfiltered marked this pull request as ready for review January 25, 2025 20:56
@webfiltered
Revert all folder_paths changes.
6b686bd 
Revert "nit - Reorder default_paths back top top of file."

This reverts commit fdfd81a.

Revert "Extend add_model_folder_path for use with built-ins."

This reverts commit a30d74d.

Revert "Reorder to keep default model paths grouped together."

This reverts commit 9c9a853.

Revert "Update type hint to reflect actual usage."

This reverts commit 74325a5.

Revert "Allow default model paths to be re-used with other base paths."

This reverts commit fa3d639.

Revert "Allow complete reset of default directories under a new base_path."

This reverts commit 6e3b261.
@webfiltered
Ensure cache is created before reorder of folder paths.
80abdea
@webfiltered
Allow folder_paths base_path to be customised.
8a29e93
@webfiltered
Update tests to match changes.
87dcae0
wibur0620
wibur0620 reviewed Jan 26, 2025
View reviewed changes
Copy link

@wibur0620 wibur0620 left a comment

Choose a reason for hiding this comment

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

👌

robinjhuang
robinjhuang requested changes Jan 28, 2025
View reviewed changes
Copy link
Collaborator

@robinjhuang robinjhuang left a comment

Choose a reason for hiding this comment

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

Synced offline. Can first make a PR with the simplest change: just import cli_args in folder_paths and set the base_path there.

@robinjhuang robinjhuang added the Good PR This PR looks good to go, it needs comfy's final review. label Jan 28, 2025
@comfyanonymous comfyanonymous merged commit 222f48c into comfyanonymous:master Jan 29, 2025
7 checks passed
@iwr-redmond iwr-redmond mentioned this pull request Jan 30, 2025
Closed
1 task
@mmartial mmartial mentioned this pull request Jan 31, 2025
Closed
@mmartial mmartial mentioned this pull request Feb 13, 2025
Closed
@polarathene polarathene mentioned this pull request May 5, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robinjhuang robinjhuang robinjhuang approved these changes

@yoland68 yoland68 Awaiting requested review from yoland68

@huchenlei huchenlei Awaiting requested review from huchenlei

@pythongosssss pythongosssss Awaiting requested review from pythongosssss

@ltdrdata ltdrdata Awaiting requested review from ltdrdata

@Kosinkadink Kosinkadink Awaiting requested review from Kosinkadink Kosinkadink is a code owner

@comfyanonymous comfyanonymous Awaiting requested review from comfyanonymous

+1 more reviewer

@wibur0620 wibur0620 wibur0620 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Good PR This PR looks good to go, it needs comfy's final review.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@webfiltered @robinjhuang @wibur0620 @comfyanonymous