Skip to content

Commit a469fd5

Browse files
committed
fix: improve type safety and error handling
1 parent 530c268 commit a469fd5

2 files changed

Lines changed: 56 additions & 7 deletions

File tree

sqlmodel/main.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -589,12 +589,12 @@ def __new__(
589589
}
590590

591591
def get_config(name: str) -> Any:
592-
config_class_value = new_cls.model_config.get(name, Undefined)
593-
if config_class_value is not Undefined:
594-
return config_class_value
595592
kwarg_value = kwargs.get(name, Undefined)
596593
if kwarg_value is not Undefined:
597594
return kwarg_value
595+
config_class_value = new_cls.model_config.get(name, Undefined)
596+
if config_class_value is not Undefined:
597+
return config_class_value
598598
return Undefined
599599

600600
config_table = get_config("table")
@@ -618,10 +618,15 @@ def get_config(name: str) -> Any:
618618
if config_registry is not Undefined:
619619
config_registry = cast(registry, config_registry)
620620
# If it was passed by kwargs, ensure it's also set in config
621-
new_cls.model_config["registry"] = config_table
622-
setattr(new_cls, "_sa_registry", config_registry) # noqa: B010
623-
setattr(new_cls, "metadata", config_registry.metadata) # noqa: B010
624-
setattr(new_cls, "__abstract__", True) # noqa: B010
621+
new_cls.model_config["registry"] = config_registry
622+
# Only set up the registry attributes when explicitly passed
623+
# as a kwarg on this class, not when inherited from a parent.
624+
# Setting __abstract__ on subclasses that merely inherit the
625+
# registry would prevent SQLAlchemy from instrumenting them.
626+
if "registry" in kwargs:
627+
setattr(new_cls, "_sa_registry", config_registry) # noqa: B010
628+
setattr(new_cls, "metadata", config_registry.metadata) # noqa: B010
629+
setattr(new_cls, "__abstract__", True) # noqa: B010
625630
return new_cls
626631

627632
# Override SQLAlchemy, allow both SQLAlchemy and plain Pydantic models

tests/test_registry.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
from sqlalchemy.orm import registry
2+
3+
from sqlmodel import Field, Session, SQLModel, create_engine, select
4+
5+
6+
def test_custom_registry_stored_in_model_config(clear_sqlmodel):
7+
"""Test that passing a custom registry via kwargs stores the registry
8+
(not the table config value) in model_config['registry'].
9+
10+
This is a regression test for a copy-paste bug where model_config['registry']
11+
was incorrectly set to the value of config_table instead of config_registry.
12+
"""
13+
custom_registry = registry()
14+
15+
class Base(SQLModel, registry=custom_registry):
16+
pass
17+
18+
class Hero(Base, table=True):
19+
id: int | None = Field(default=None, primary_key=True)
20+
name: str
21+
22+
# The registry stored in model_config should be the custom registry,
23+
# not a bool (which config_table would be)
24+
assert Base.model_config.get("registry") is custom_registry
25+
assert isinstance(Base.model_config.get("registry"), registry)
26+
27+
# Verify the custom registry is actually functional
28+
engine = create_engine("sqlite://")
29+
custom_registry.metadata.create_all(engine)
30+
31+
with Session(engine) as session:
32+
hero = Hero(name="Spider-Boy")
33+
session.add(hero)
34+
session.commit()
35+
session.refresh(hero)
36+
assert hero.id is not None
37+
assert hero.name == "Spider-Boy"
38+
39+
with Session(engine) as session:
40+
heroes = session.exec(select(Hero)).all()
41+
assert len(heroes) == 1
42+
assert heroes[0].name == "Spider-Boy"
43+
44+
custom_registry.dispose()

0 commit comments

Comments
 (0)