Skip to content

fix: Missing data dirs should be handled gracefully #8115

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: Missing data dirs should be handled gracefully #8115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
415cb80
fix: Should gracefully handle missing dirs
polarathene May 14, 2025
edc85f0
chore: Co-locate `args.temp_directory`
polarathene May 14, 2025
5b1bbaf
chore: Co-locate `--user-directory` arg
polarathene May 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions comfy/cli_args.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ def __call__(self, parser, namespace, values, option_string=None):

parser.add_argument("--base-directory", type=str, default=None, help="Set the ComfyUI base directory for models, custom_nodes, input, output, temp, and user directories.")
parser.add_argument("--extra-model-paths-config", type=str, default=None, metavar="PATH", nargs='+', action='append', help="Load one or more extra_model_paths.yaml files.")
parser.add_argument("--input-directory", type=str, default=None, help="Set the ComfyUI input directory. Overrides --base-directory.")
parser.add_argument("--output-directory", type=str, default=None, help="Set the ComfyUI output directory. Overrides --base-directory.")
parser.add_argument("--temp-directory", type=str, default=None, help="Set the ComfyUI temp directory (default is in the ComfyUI directory). Overrides --base-directory.")
parser.add_argument("--input-directory", type=str, default=None, help="Set the ComfyUI input directory. Overrides --base-directory.")
parser.add_argument("--user-directory", type=is_valid_directory, default=None, help="Set the ComfyUI user directory with an absolute path. Overrides --base-directory.")
Copy link
Author

Choose a reason for hiding this comment

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

For consistency, could set this to str and it'll be created if it's missing, otherwise the other related options could also use type=is_valid_directory to enforce a valid dir is provided (but that may break some expectations?)

Suggested change
parser.add_argument("--user-directory", type=is_valid_directory, default=None, help="Set the ComfyUI user directory with an absolute path. Overrides --base-directory.")
parser.add_argument("--user-directory", type=str, default=None, help="Set the ComfyUI user directory with an absolute path. Overrides --base-directory.")

parser.add_argument("--auto-launch", action="store_true", help="Automatically launch ComfyUI in the default browser.")
parser.add_argument("--disable-auto-launch", action="store_true", help="Disable auto launching the browser.")
parser.add_argument("--cuda-device", type=int, default=None, metavar="DEVICE_ID", help="Set the id of the cuda device this instance will use.")
Expand Down Expand Up @@ -191,8 +192,6 @@ def is_valid_directory(path: str) -> str:
help="The local filesystem path to the directory where the frontend is located. Overrides --front-end-version.",
)

parser.add_argument("--user-directory", type=is_valid_directory, default=None, help="Set the ComfyUI user directory with an absolute path. Overrides --base-directory.")

parser.add_argument("--enable-compress-response-body", action="store_true", help="Enable compressing response body.")

parser.add_argument(
Expand Down
6 changes: 0 additions & 6 deletions folder_paths.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,6 @@ def map_legacy(folder_name: str) -> str:
"clip": "text_encoders"}
return legacy.get(folder_name, folder_name)

if not os.path.exists(input_directory):
try:
os.makedirs(input_directory)
except:
logging.error("Failed to create input directory")

def set_output_directory(output_dir: str) -> None:
global output_directory
output_directory = output_dir
Expand Down
51 changes: 36 additions & 15 deletions main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,22 @@ def apply_custom_paths():
for config_path in itertools.chain(*args.extra_model_paths_config):
utils.extra_config.load_extra_path_config(config_path)

# --output-directory, --input-directory, --user-directory
# --temp-directory, --user-directory, --input-directory, --output-directory
if args.temp_directory:
temp_dir = os.path.join(os.path.abspath(args.temp_directory), "temp")
logging.info(f"Setting temp directory to: {temp_dir}")
folder_paths.set_temp_directory(temp_dir)
Comment on lines +34 to +37
Copy link
Author

Choose a reason for hiding this comment

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

Breaking change, but would be consistent with expectations of related args:

Suggested change
if args.temp_directory:
temp_dir = os.path.join(os.path.abspath(args.temp_directory), "temp")
logging.info(f"Setting temp directory to: {temp_dir}")
folder_paths.set_temp_directory(temp_dir)
if args.temp_directory:
temp_dir = os.path.abspath(args.temp_directory)
logging.info(f"Setting temp directory to: {temp_dir}")
folder_paths.set_temp_directory(temp_dir)


if args.user_directory:
user_dir = os.path.abspath(args.user_directory)
logging.info(f"Setting user directory to: {user_dir}")
folder_paths.set_user_directory(user_dir)

if args.input_directory:
input_dir = os.path.abspath(args.input_directory)
logging.info(f"Setting input directory to: {input_dir}")
folder_paths.set_input_directory(input_dir)

if args.output_directory:
output_dir = os.path.abspath(args.output_directory)
logging.info(f"Setting output directory to: {output_dir}")
Expand All @@ -44,15 +59,20 @@ def apply_custom_paths():
os.path.join(folder_paths.get_output_directory(), "diffusion_models"))
folder_paths.add_model_folder_path("loras", os.path.join(folder_paths.get_output_directory(), "loras"))

if args.input_directory:
input_dir = os.path.abspath(args.input_directory)
logging.info(f"Setting input directory to: {input_dir}")
folder_paths.set_input_directory(input_dir)

if args.user_directory:
user_dir = os.path.abspath(args.user_directory)
logging.info(f"Setting user directory to: {user_dir}")
folder_paths.set_user_directory(user_dir)
# These are created at startup if missing. Other data directories are created on-demand when needed.
def create_data_folders():
data_dirs = [
folder_paths.get_input_directory(),
folder_paths.get_temp_directory()
]
Comment on lines +65 to +68
Copy link
Author

Choose a reason for hiding this comment

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

Optionally create the user/ directory here (UserManager already does so at startup?), and restore the output/ directory creation (presumably not necessary and is instead handled on-demand).

Suggested change
data_dirs = [
folder_paths.get_input_directory(),
folder_paths.get_temp_directory()
]
data_dirs = [
folder_paths.get_input_directory(),
folder_paths.get_output_directory(),
folder_paths.get_temp_directory(),
folder_paths.get_user_directory()
]


for dir_path in data_dirs:
if not os.path.exists(dir_path):
try:
os.makedirs(dir_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe adding exist_ok=True here will be a good idea

Copy link
Author

@polarathene polarathene May 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't see why?

  • There is a condition already not os.path.exists, which would make exist_ok=True redundant.
  • While you could also say that exist_ok=True would make the need for the not os.path.exists condition itself redundant - if this were a read-only file system (and a directory that doesn't actually require writing into at runtime) then enforcing a write via exist_ok=True would be incompatible.

Hence this approach is better, only attempt to create a directory when it doesn't exist but is expected to.

except:
logging.error(f"Failed to create directory: {dir_path}")


def execute_prestartup_script():
Expand All @@ -72,7 +92,11 @@ def execute_script(script_path):

node_paths = folder_paths.get_folder_paths("custom_nodes")
for custom_node_path in node_paths:
possible_modules = os.listdir(custom_node_path)
if not os.path.exists(custom_node_path):
logging.warning(f"Directory {custom_node_path} for custom nodes does not exist, skipping")
continue
Comment on lines +95 to +97
Copy link
Author

Choose a reason for hiding this comment

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

This is the primary fix I'm interested in, if the directory does not need to exist but is checked by default, it seems better to skip instead of crash?

I am not sure if that aligns with your expectations, so if it makes sense to log a warning I've added one here, otherwise the warning could be removed?


possible_modules = os.listdir(os.path.realpath(custom_node_path))
Copy link
Author

Choose a reason for hiding this comment

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

os.path.realpath was added here but could be reverted.

A user reported the lack of resolving symlinks as problematic for similar custom_node_path logic at nodes.py and resolved it by adding os.path.realpath: #2035

I added it here for consistency.

node_prestartup_times = []

for possible_module in possible_modules:
Expand All @@ -95,7 +119,9 @@ def execute_script(script_path):
logging.info("{:6.1f} seconds{}: {}".format(n[0], import_message, n[1]))
logging.info("")


apply_custom_paths()
create_data_folders()
execute_prestartup_script()


Expand Down Expand Up @@ -250,10 +276,6 @@ def start_comfyui(asyncio_loop=None):
Starts the ComfyUI server using the provided asyncio event loop or creates a new one.
Returns the event loop, server instance, and a function to start the server asynchronously.
"""
if args.temp_directory:
temp_dir = os.path.join(os.path.abspath(args.temp_directory), "temp")
logging.info(f"Setting temp directory to: {temp_dir}")
folder_paths.set_temp_directory(temp_dir)
cleanup_temp()

if args.windows_standalone_build:
Expand Down Expand Up @@ -283,7 +305,6 @@ def start_comfyui(asyncio_loop=None):
if args.quick_test_for_ci:
exit(0)

os.makedirs(folder_paths.get_temp_directory(), exist_ok=True)
call_on_start = None
if args.auto_launch:
def startup_server(scheme, address, port):
Expand Down