From 9789de07b88713e7a3ce7b2948b26cc9f419b31e Mon Sep 17 00:00:00 2001 From: Constantine1916 Date: Thu, 18 Jun 2026 21:42:58 +0800 Subject: [PATCH 1/3] fix: respect user directory for default database --- app/database/db.py | 52 +++++++++++++-- comfy/cli_args.py | 6 ++ tests-unit/app_test/database_path_test.py | 78 +++++++++++++++++++++++ 3 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 tests-unit/app_test/database_path_test.py diff --git a/app/database/db.py b/app/database/db.py index 0aab09a49..ab2198814 100644 --- a/app/database/db.py +++ b/app/database/db.py @@ -57,19 +57,62 @@ def get_alembic_config(): config = Config(config_path) config.set_main_option("script_location", scripts_path) - config.set_main_option("sqlalchemy.url", args.database_url) + config.set_main_option("sqlalchemy.url", get_database_url()) return config -def get_db_path(): +def get_database_url(): + if getattr(args, "database_url_explicit", False): + return args.database_url + + import folder_paths + + db_path = os.path.join(folder_paths.get_user_directory(), "comfyui.db") + return f"sqlite:///{db_path}" + + +def get_legacy_default_db_path(): url = args.database_url if url.startswith("sqlite:///"): - return url.split("///")[1] + return url.split("///", 1)[1] + return None + + +def get_db_path(): + url = get_database_url() + if url.startswith("sqlite:///"): + return url.split("///", 1)[1] else: raise ValueError(f"Unsupported database URL '{url}'.") +def copy_legacy_default_db(db_path): + if getattr(args, "database_url_explicit", False): + return + + legacy_db_path = get_legacy_default_db_path() + if legacy_db_path is None: + return + + if os.path.abspath(legacy_db_path) == os.path.abspath(db_path): + return + + if os.path.exists(db_path) or not os.path.exists(legacy_db_path): + return + + shutil.copy(legacy_db_path, db_path) + logging.info(f"Copied legacy database from '{legacy_db_path}' to '{db_path}'") + + +def prepare_file_db_path(db_path): + db_dir = os.path.dirname(db_path) + if db_dir: + os.makedirs(db_dir, exist_ok=True) + + copy_legacy_default_db(db_path) + + _db_lock = None def _acquire_file_lock(db_path): @@ -97,7 +140,7 @@ def _is_memory_db(db_url): def init_db(): - db_url = args.database_url + db_url = get_database_url() logging.debug(f"Database URL: {db_url}") if _is_memory_db(db_url): @@ -134,6 +177,7 @@ def _init_memory_db(db_url): def _init_file_db(db_url): """Initialize a file-backed SQLite database using Alembic migrations.""" db_path = get_db_path() + prepare_file_db_path(db_path) db_exists = os.path.exists(db_path) config = get_alembic_config() diff --git a/comfy/cli_args.py b/comfy/cli_args.py index e3099a230..705125008 100644 --- a/comfy/cli_args.py +++ b/comfy/cli_args.py @@ -1,6 +1,7 @@ import argparse import enum import os +import sys import comfy.options @@ -245,8 +246,13 @@ parser.add_argument("--list-feature-flags", action="store_true", help="Print the if comfy.options.args_parsing: args = parser.parse_args() + args.database_url_explicit = any( + arg == "--database-url" or arg.startswith("--database-url=") + for arg in sys.argv[1:] + ) else: args = parser.parse_args([]) + args.database_url_explicit = False if args.cache_ram is not None and len(args.cache_ram) > 2: parser.error("--cache-ram accepts at most two values: active GB and inactive GB") diff --git a/tests-unit/app_test/database_path_test.py b/tests-unit/app_test/database_path_test.py new file mode 100644 index 000000000..52b1b6b99 --- /dev/null +++ b/tests-unit/app_test/database_path_test.py @@ -0,0 +1,78 @@ +import os + +from app.database import db + + +def test_default_database_url_uses_effective_user_directory(monkeypatch, tmp_path): + user_dir = tmp_path / "custom_user" + user_dir.mkdir() + + monkeypatch.setattr(db.args, "database_url_explicit", False, raising=False) + monkeypatch.setattr("folder_paths.get_user_directory", lambda: str(user_dir)) + + assert db.get_database_url() == f"sqlite:///{user_dir / 'comfyui.db'}" + + +def test_explicit_database_url_is_preserved(monkeypatch): + database_url = "sqlite:///:memory:" + + monkeypatch.setattr(db.args, "database_url", database_url) + monkeypatch.setattr(db.args, "database_url_explicit", True, raising=False) + + assert db.get_database_url() == database_url + + +def test_legacy_default_database_is_copied_to_effective_user_directory(monkeypatch, tmp_path): + legacy_db = tmp_path / "install" / "user" / "comfyui.db" + user_dir = tmp_path / "custom_user" + legacy_db.parent.mkdir(parents=True) + user_dir.mkdir() + legacy_db.write_bytes(b"legacy db") + + monkeypatch.setattr(db.args, "database_url_explicit", False, raising=False) + monkeypatch.setattr("folder_paths.get_user_directory", lambda: str(user_dir)) + monkeypatch.setattr(db, "get_legacy_default_db_path", lambda: str(legacy_db)) + + db.copy_legacy_default_db(str(user_dir / "comfyui.db")) + + assert (user_dir / "comfyui.db").read_bytes() == b"legacy db" + assert legacy_db.read_bytes() == b"legacy db" + + +def test_legacy_default_database_does_not_overwrite_existing_effective_db(monkeypatch, tmp_path): + legacy_db = tmp_path / "install" / "user" / "comfyui.db" + user_db = tmp_path / "custom_user" / "comfyui.db" + legacy_db.parent.mkdir(parents=True) + user_db.parent.mkdir(parents=True) + legacy_db.write_bytes(b"legacy db") + user_db.write_bytes(b"user db") + + monkeypatch.setattr(db.args, "database_url_explicit", False, raising=False) + monkeypatch.setattr(db, "get_legacy_default_db_path", lambda: str(legacy_db)) + + db.copy_legacy_default_db(str(user_db)) + + assert user_db.read_bytes() == b"user db" + assert legacy_db.read_bytes() == b"legacy db" + + +def test_prepare_file_database_creates_parent_directory(monkeypatch, tmp_path): + db_path = tmp_path / "nested" / "comfyui.db" + + monkeypatch.setattr(db.args, "database_url_explicit", False, raising=False) + monkeypatch.setattr(db, "copy_legacy_default_db", lambda path: None) + + db.prepare_file_db_path(str(db_path)) + + assert db_path.parent.is_dir() + + +def test_prepare_file_database_accepts_relative_database_path(monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(db.args, "database_url_explicit", True, raising=False) + monkeypatch.setattr(db, "copy_legacy_default_db", lambda path: None) + + db.prepare_file_db_path("relative.db") + + assert os.getcwd() == str(tmp_path) + assert not list(tmp_path.iterdir()) From c34b704f94355a790f85912bd862641aba917c31 Mon Sep 17 00:00:00 2001 From: Constantine Date: Sat, 11 Jul 2026 22:02:20 +0800 Subject: [PATCH 2/3] refactor: use None default for --database-url instead of argv scan Per review: default --database-url to None and treat a non-None value as explicit at resolution time. Removes the sys.argv scan and the database_url_explicit attribute. database_default_path now serves directly as the legacy copy source. Adds regression tests for the unchanged no-flag default path and explicit URLs at the legacy location. --- app/database/db.py | 11 +++--- comfy/cli_args.py | 8 +--- tests-unit/app_test/database_path_test.py | 48 ++++++++++++++++++++--- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/app/database/db.py b/app/database/db.py index ab2198814..0377c8dfb 100644 --- a/app/database/db.py +++ b/app/database/db.py @@ -63,7 +63,7 @@ def get_alembic_config(): def get_database_url(): - if getattr(args, "database_url_explicit", False): + if args.database_url is not None: return args.database_url import folder_paths @@ -73,10 +73,9 @@ def get_database_url(): def get_legacy_default_db_path(): - url = args.database_url - if url.startswith("sqlite:///"): - return url.split("///", 1)[1] - return None + from comfy.cli_args import database_default_path + + return database_default_path def get_db_path(): @@ -88,7 +87,7 @@ def get_db_path(): def copy_legacy_default_db(db_path): - if getattr(args, "database_url_explicit", False): + if args.database_url is not None: return legacy_db_path = get_legacy_default_db_path() diff --git a/comfy/cli_args.py b/comfy/cli_args.py index 705125008..8190be2ce 100644 --- a/comfy/cli_args.py +++ b/comfy/cli_args.py @@ -1,7 +1,6 @@ import argparse import enum import os -import sys import comfy.options @@ -239,20 +238,15 @@ parser.add_argument( database_default_path = os.path.abspath( os.path.join(os.path.dirname(__file__), "..", "user", "comfyui.db") ) -parser.add_argument("--database-url", type=str, default=f"sqlite:///{database_default_path}", help="Specify the database URL, e.g. for an in-memory database you can use 'sqlite:///:memory:'.") +parser.add_argument("--database-url", type=str, default=None, help="Specify the database URL, e.g. for an in-memory database you can use 'sqlite:///:memory:'. Defaults to 'comfyui.db' in the effective user directory.") parser.add_argument("--enable-assets", action="store_true", help="Enable the assets system (API routes, database synchronization, and background scanning).") parser.add_argument("--feature-flag", type=str, action='append', default=[], metavar="KEY[=VALUE]", help="Set a server feature flag. Use KEY=VALUE to set an explicit value, or bare KEY to set it to true. Can be specified multiple times. Boolean values (true/false) and numbers are auto-converted. Examples: --feature-flag show_signin_button=true or --feature-flag show_signin_button") parser.add_argument("--list-feature-flags", action="store_true", help="Print the registry of known CLI-settable feature flags as JSON and exit.") if comfy.options.args_parsing: args = parser.parse_args() - args.database_url_explicit = any( - arg == "--database-url" or arg.startswith("--database-url=") - for arg in sys.argv[1:] - ) else: args = parser.parse_args([]) - args.database_url_explicit = False if args.cache_ram is not None and len(args.cache_ram) > 2: parser.error("--cache-ram accepts at most two values: active GB and inactive GB") diff --git a/tests-unit/app_test/database_path_test.py b/tests-unit/app_test/database_path_test.py index 52b1b6b99..543c0ba10 100644 --- a/tests-unit/app_test/database_path_test.py +++ b/tests-unit/app_test/database_path_test.py @@ -1,27 +1,63 @@ import os from app.database import db +from comfy.cli_args import database_default_path def test_default_database_url_uses_effective_user_directory(monkeypatch, tmp_path): user_dir = tmp_path / "custom_user" user_dir.mkdir() - monkeypatch.setattr(db.args, "database_url_explicit", False, raising=False) + monkeypatch.setattr(db.args, "database_url", None) monkeypatch.setattr("folder_paths.get_user_directory", lambda: str(user_dir)) assert db.get_database_url() == f"sqlite:///{user_dir / 'comfyui.db'}" +def test_default_db_path_matches_legacy_default_without_custom_user_directory(monkeypatch): + import folder_paths + + monkeypatch.setattr(db.args, "database_url", None) + monkeypatch.setattr( + "folder_paths.get_user_directory", + lambda: os.path.join(folder_paths.base_path, "user"), + ) + + assert os.path.abspath(db.get_db_path()) == database_default_path + + def test_explicit_database_url_is_preserved(monkeypatch): database_url = "sqlite:///:memory:" monkeypatch.setattr(db.args, "database_url", database_url) - monkeypatch.setattr(db.args, "database_url_explicit", True, raising=False) assert db.get_database_url() == database_url +def test_explicit_url_at_legacy_default_location_is_honoured(monkeypatch): + url = f"sqlite:///{database_default_path}" + + monkeypatch.setattr(db.args, "database_url", url) + + assert db.get_database_url() == url + assert db.get_db_path() == database_default_path + + +def test_explicit_database_url_skips_legacy_copy(monkeypatch, tmp_path): + legacy_db = tmp_path / "install" / "user" / "comfyui.db" + target_db = tmp_path / "target" / "comfyui.db" + legacy_db.parent.mkdir(parents=True) + target_db.parent.mkdir(parents=True) + legacy_db.write_bytes(b"legacy db") + + monkeypatch.setattr(db.args, "database_url", f"sqlite:///{target_db}") + monkeypatch.setattr(db, "get_legacy_default_db_path", lambda: str(legacy_db)) + + db.copy_legacy_default_db(str(target_db)) + + assert not target_db.exists() + + def test_legacy_default_database_is_copied_to_effective_user_directory(monkeypatch, tmp_path): legacy_db = tmp_path / "install" / "user" / "comfyui.db" user_dir = tmp_path / "custom_user" @@ -29,7 +65,7 @@ def test_legacy_default_database_is_copied_to_effective_user_directory(monkeypat user_dir.mkdir() legacy_db.write_bytes(b"legacy db") - monkeypatch.setattr(db.args, "database_url_explicit", False, raising=False) + monkeypatch.setattr(db.args, "database_url", None) monkeypatch.setattr("folder_paths.get_user_directory", lambda: str(user_dir)) monkeypatch.setattr(db, "get_legacy_default_db_path", lambda: str(legacy_db)) @@ -47,7 +83,7 @@ def test_legacy_default_database_does_not_overwrite_existing_effective_db(monkey legacy_db.write_bytes(b"legacy db") user_db.write_bytes(b"user db") - monkeypatch.setattr(db.args, "database_url_explicit", False, raising=False) + monkeypatch.setattr(db.args, "database_url", None) monkeypatch.setattr(db, "get_legacy_default_db_path", lambda: str(legacy_db)) db.copy_legacy_default_db(str(user_db)) @@ -59,7 +95,7 @@ def test_legacy_default_database_does_not_overwrite_existing_effective_db(monkey def test_prepare_file_database_creates_parent_directory(monkeypatch, tmp_path): db_path = tmp_path / "nested" / "comfyui.db" - monkeypatch.setattr(db.args, "database_url_explicit", False, raising=False) + monkeypatch.setattr(db.args, "database_url", None) monkeypatch.setattr(db, "copy_legacy_default_db", lambda path: None) db.prepare_file_db_path(str(db_path)) @@ -69,7 +105,7 @@ def test_prepare_file_database_creates_parent_directory(monkeypatch, tmp_path): def test_prepare_file_database_accepts_relative_database_path(monkeypatch, tmp_path): monkeypatch.chdir(tmp_path) - monkeypatch.setattr(db.args, "database_url_explicit", True, raising=False) + monkeypatch.setattr(db.args, "database_url", "sqlite:///relative.db") monkeypatch.setattr(db, "copy_legacy_default_db", lambda path: None) db.prepare_file_db_path("relative.db") From 2e1235222e71178460b70f30e48ecfbc876276a7 Mon Sep 17 00:00:00 2001 From: Constantine Date: Tue, 14 Jul 2026 12:25:52 +0800 Subject: [PATCH 3/3] refactor: rename legacy database to .bak after copy Per review: after copying the legacy install-dir database to the effective user directory, rename the original to comfyui.db.bak so a later launch without --user-directory cannot silently fall back to a diverged copy, while keeping the file around for recovery. Also hoist the database_default_path import to module scope. --- app/database/db.py | 9 +++++---- tests-unit/app_test/database_path_test.py | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/database/db.py b/app/database/db.py index 0377c8dfb..850a28c9c 100644 --- a/app/database/db.py +++ b/app/database/db.py @@ -4,7 +4,7 @@ import shutil from app.logger import log_startup_warning from utils.install_util import get_missing_requirements_message from filelock import FileLock, Timeout -from comfy.cli_args import args +from comfy.cli_args import args, database_default_path _DB_AVAILABLE = False Session = None @@ -73,8 +73,6 @@ def get_database_url(): def get_legacy_default_db_path(): - from comfy.cli_args import database_default_path - return database_default_path @@ -101,7 +99,10 @@ def copy_legacy_default_db(db_path): return shutil.copy(legacy_db_path, db_path) - logging.info(f"Copied legacy database from '{legacy_db_path}' to '{db_path}'") + os.replace(legacy_db_path, legacy_db_path + ".bak") + logging.info( + f"Copied legacy database from '{legacy_db_path}' to '{db_path}' and renamed the original to '{legacy_db_path}.bak'" + ) def prepare_file_db_path(db_path): diff --git a/tests-unit/app_test/database_path_test.py b/tests-unit/app_test/database_path_test.py index 543c0ba10..7966d700e 100644 --- a/tests-unit/app_test/database_path_test.py +++ b/tests-unit/app_test/database_path_test.py @@ -72,7 +72,8 @@ def test_legacy_default_database_is_copied_to_effective_user_directory(monkeypat db.copy_legacy_default_db(str(user_dir / "comfyui.db")) assert (user_dir / "comfyui.db").read_bytes() == b"legacy db" - assert legacy_db.read_bytes() == b"legacy db" + assert not legacy_db.exists() + assert legacy_db.with_suffix(".db.bak").read_bytes() == b"legacy db" def test_legacy_default_database_does_not_overwrite_existing_effective_db(monkeypatch, tmp_path):