Add max depth parameter to be able load nested relationships on a model #44

bharadwajyarlagadda · 2025-11-18T20:20:04Z

Problem

Right now, to_dict method goes into recursion issue whenever we want to load all the nested relationships
We also have the issue where if we have multiple attribute/relationships with the same model, only one attribute gets loaded with to_dict and the other doesn't load as the model is already in seen and is not going to be visited again

Goal

closes ModelSerializer.to_dict method not loading multiple relationships with the same model #43

Approach Taken

Introduced a max_depth parameter which can help only load relationship upto a certain depth
I also went ahead and introduced cache where we can store the model object based on the id value. So, when a model is already visited and if we want it again - we can load it up from the cache

…ionships

dgilland · 2025-11-20T00:43:41Z

src/sqlservice/model.py

        By default, only table columns will be included. To include relationship fields, set
        ``include_relationships=True``. This will nest ``to_dict()`` calls to the relationship
        models.
+
+        ``exclude_nested_relationships``: Exclude nested relationships. Defaults to ``False``.
+        ``include_nested_relationships``: Include nested relationships. Defaults to ``False``.


Looks like I have some stale docs with references to include_relationships as a parameter and incorrect behavior description. Mind updating that to say something about how relationships are included by default but can be excluded with exclude_relationships=True? Can also add these new fields in the description as well but within the paragraph instead of as a separate line items.

dgilland · 2025-11-20T00:47:25Z

src/sqlservice/model.py

+        exclude_nested_relationships: bool = False,
+        include_nested_relationships: bool = False,


I don't think both of these are needed since they should be mutually exclusive (i.e. one wouldn't set exclude_nested_relationships=True and include_nested_relationships=True). If the default behavior is to include nested relationships, then should use exclude_nested_relationships but if the default behavior is to excluded nested relationships, then should use include_nested_relationships.

I'm leaning towards having the nested relationships excluded by default so would just have include_nested_relationships. Thought?

dgilland · 2025-11-20T01:09:26Z

tests/test_model.py

+                "created_by_user": {"id": 1, "name": "n", "active": True},
+                "updated_by_user": {"id": 2, "name": "n", "active": True},
+            },
+            id="circular_reference_lazy_loaded",


Suggested change

id="circular_reference_lazy_loaded",

id="exclude_nested_relationships_lazy_loaded_with_circular_reference",

dgilland · 2025-11-20T01:10:10Z

tests/test_model.py

+                    ],
+                },
+            },
+            id="circular_reference_lazy_loaded_with_max_depth",


Suggested change

id="circular_reference_lazy_loaded_with_max_depth",

id="include_nested_relationships_lazy_loaded_with_circular_reference",

dgilland · 2025-11-20T01:13:25Z

tests/test_model.py

+                created_by_user=User(id=1, name="n"),
+                updated_by_user=User(id=2, name="n"),
+            ),
+            {"lazyload": True, "include_nested_relationships": True},


Would be good to also have tests without lazy-loading that verify that non-loaded nested relationships won't be included.

dgilland · 2025-11-20T01:15:48Z

tests/fixtures.py

+    title: Mapped[str] = mapped_column(sa.String())
+    created_by: Mapped[int] = mapped_column(sa.Integer(), sa.ForeignKey("users.id"), nullable=False)
+    created_by_user: Mapped["User"] = relationship(
+        "User", foreign_keys=[created_by], back_populates="created_books"


For my own knowledge since I never really used back_populates: Is back_populates needed here when the corresponding field is explicitly defined in the other model? Could that argument be dropped from all the fields or is it needed? What's the behavior if it's left off?

…ome stale docs

coveralls · 2025-11-20T02:31:05Z

coverage: 100.0%. remained the same
when pulling 77b93c5 on bharadwajyarlagadda:43-add-max-depth-base-model
into 7e091c8 on dgilland:master.

dgilland

Just some nits on organization and docstring wording.

dgilland · 2025-11-20T02:29:09Z

src/sqlservice/model.py

+        exclude_relationships: bool = False,
+        lazyload: bool = False,
+        include_nested_relationships: bool = False,


Group relationship arguments together:

Suggested change

exclude_relationships: bool = False,

lazyload: bool = False,

include_nested_relationships: bool = False,

lazyload: bool = False,

exclude_relationships: bool = False,

include_nested_relationships: bool = False,

dgilland · 2025-11-20T02:31:26Z

src/sqlservice/model.py

+        By default, only table columns will be included. To exclude relationships, set
+        ``exclude_relationships=True``.
+
+        To include nested relationships, set ``include_nested_relationships=True``.


Suggested change

By default, only table columns will be included. To exclude relationships, set

``exclude_relationships=True``.

To include nested relationships, set ``include_nested_relationships=True``.

By default, table columns and relationships will be included while nested relationships will be excluded. To exclude relationships, set ``exclude_relationships=True``. To include nested relationships, set ``include_nested_relationships=True``.

dgilland · 2025-11-20T02:31:58Z

src/sqlservice/model.py

+            exclude_relationships=exclude_relationships,
+            lazyload=lazyload,
+            include_nested_relationships=include_nested_relationships,


Group relationship arguments together:

Suggested change

exclude_relationships=exclude_relationships,

lazyload=lazyload,

include_nested_relationships=include_nested_relationships,

lazyload=lazyload,

exclude_relationships=exclude_relationships,

include_nested_relationships=include_nested_relationships,

dgilland · 2025-11-20T02:33:04Z

src/sqlservice/model.py

+        exclude_relationships: bool = False,
+        lazyload: bool = False,
+        include_nested_relationships: bool = False,
+    ):
        self.exclude_relationships = exclude_relationships
        self.lazyload = lazyload
+        self.include_nested_relationships = include_nested_relationships


Group relationship arguments together:

Suggested change

exclude_relationships: bool = False,

lazyload: bool = False,

include_nested_relationships: bool = False,

):

self.exclude_relationships = exclude_relationships

self.lazyload = lazyload

self.include_nested_relationships = include_nested_relationships

lazyload: bool = False,

exclude_relationships: bool = False,

include_nested_relationships: bool = False,

):

self.lazyload = lazyload

self.exclude_relationships = exclude_relationships

self.include_nested_relationships = include_nested_relationships

Bharadwaj Yarlagadda and others added 2 commits November 19, 2025 18:13

Add max depth parameter to be able load nested relationships on a model

6d78778

Replace the max_depth with flags for including/excluding nested relat…

775190f

…ionships

bharadwajyarlagadda force-pushed the 43-add-max-depth-base-model branch from a4d4795 to 775190f Compare November 19, 2025 23:30

dgilland reviewed Nov 20, 2025

View reviewed changes

Removed exclude_nested_relationships from ModelSerializer and fixed s…

222cf6c

…ome stale docs

dgilland reviewed Nov 20, 2025

View reviewed changes

Grouped the relationship related arguments together in ModelSerializer

77b93c5

dgilland merged commit 6872646 into dgilland:master Nov 20, 2025
14 checks passed

		exclude_nested_relationships: bool = False,
		include_nested_relationships: bool = False,

	id="circular_reference_lazy_loaded",
	id="exclude_nested_relationships_lazy_loaded_with_circular_reference",

	id="circular_reference_lazy_loaded_with_max_depth",
	id="include_nested_relationships_lazy_loaded_with_circular_reference",

Add max depth parameter to be able load nested relationships on a model #44

Add max depth parameter to be able load nested relationships on a model #44

Uh oh!

Conversation

bharadwajyarlagadda commented Nov 18, 2025

Problem

Goal

Approach Taken

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dgilland left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented Nov 20, 2025 •

edited

Loading