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.
This commit is contained in:
Constantine
parent
9789de07b8
commit
c34b704f94
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
Loading…
Reference in New Issue