FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

✨ Support SQLAlchemy keyed dict models by nkrishnaswami · Pull Request #1287 · fastapi/sqlmodel · GitHub

  1. fastapi
  2. sqlmodel

Repository navigation

✨ Support SQLAlchemy keyed dict models - #1287

Open
nkrishnaswami wants to merge 22 commits into
fastapi:mainfrom
nkrishnaswami:allow-dict-relationship
ConversationCommits (22)ChecksFiles changed
Open

✨ Support SQLAlchemy keyed dict models#1287
nkrishnaswami wants to merge 22 commits into
fastapi:mainfrom
nkrishnaswami:allow-dict-relationship

Conversation

nkrishnaswami commented Feb 6, 2025 •
edited
Loading

Copy link
Copy Markdown

To add support for SQLAlchemy mapped collections, I'd like to add a case for dict origins to the Pydantic v2 get_relationship_to implementation. This PR does so, with a test case using a dictionary with collection class attribute_keyed_dict.

svlandeg changed the title Minimal support for SQLAlchemy keyed dict models ✨ Support SQLAlchemy keyed dict models Feb 20, 2025
svlandeg added the feature New feature or request label Feb 20, 2025

nkrishnaswami commented Feb 20, 2025

Copy link
Copy Markdown
Author

Thanks, @svlandeg!

dcode commented Feb 26, 2025

Copy link
Copy Markdown

Any update on this? This is a great feature add that I need. For now, I'm setting the requirement to use git at this PR.

nkrishnaswami commented Mar 17, 2025

Copy link
Copy Markdown
Author

@svlandeg what is the next step in the process? How do I get this to be reviewed? (Thanks!)

svlandeg commented Mar 18, 2025

Copy link
Copy Markdown
Member

Hi @nkrishnaswami! It's in our review queue, so not much to be done by you at this point in time. We'll get back to you once we've had time to go through this in details 🙏

saky-semicolon reviewed Sep 26, 2025
View reviewed changes

saky-semicolon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good addition — the change in _compat.py improves handling of dict annotations, and the new test (test_attribute_keyed_dict.py) clearly validates the functionality with attribute_keyed_dict.

YuriiMotov approved these changes Sep 30, 2025
View reviewed changes

YuriiMotov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good!

We should probably re-export sqlalchemy.orm.collections

Comment thread tests/test_attribute_keyed_dict.py Outdated
YuriiMotov mentioned this pull request Oct 29, 2025
Closed
YuriiMotov reviewed Oct 29, 2025
View reviewed changes
Comment thread sqlmodel/_compat.py Outdated
nkrishnaswami force-pushed the allow-dict-relationship branch from c7c1402 to a801312 Compare November 3, 2025 23:28
YuriiMotov reviewed Nov 4, 2025
View reviewed changes

YuriiMotov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@nkrishnaswami, thank you!

Could you please address my review comments?

Comment thread tests/test_attribute_keyed_dict.py Outdated
Comment thread sqlmodel/_compat.py Outdated
Comment thread tests/test_attribute_keyed_dict.py Outdated
YuriiMotov reviewed Nov 4, 2025
View reviewed changes
Comment thread tests/test_attribute_keyed_dict.py Outdated
Comment thread tests/test_attribute_keyed_dict.py Outdated
Comment thread tests/test_attribute_keyed_dict.py Outdated
nkrishnaswami changed the title ✨ Support SQLAlchemy keyed dict models ✨ Support SQLAlchemy keyed dict models with Pydantic v2 Nov 5, 2025
YuriiMotov approved these changes Nov 5, 2025
View reviewed changes

YuriiMotov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM!

@nkrishnaswami, thank you!

nkrishnaswami commented Nov 5, 2025

Copy link
Copy Markdown
Author

YW, and thanks for reviewing it!

github-actions Bot added the conflicts Automatically generated when a PR has a merge conflict label Dec 28, 2025

This comment was marked as resolved.

Sign in to view
nkrishnaswami force-pushed the allow-dict-relationship branch from 1808d25 to 3249f9b Compare December 28, 2025 19:39
github-actions Bot removed the conflicts Automatically generated when a PR has a merge conflict label Dec 28, 2025
nkrishnaswami force-pushed the allow-dict-relationship branch from 4732119 to 8c81116 Compare December 28, 2025 19:53
YuriiMotov changed the title ✨ Support SQLAlchemy keyed dict models with Pydantic v2 ✨ Support SQLAlchemy keyed dict models Jan 26, 2026

nkrishnaswami commented Feb 11, 2026

Copy link
Copy Markdown
Author

What is the next step after approval?

nkrishnaswami force-pushed the allow-dict-relationship branch from 8c81116 to 1bf3b91 Compare February 11, 2026 16:25

YuriiMotov commented Feb 11, 2026

Copy link
Copy Markdown
Member

What is the next step after approval?

This PR is in the queue for the final review by Sebastian

apiwat-chantawibul reviewed Jun 1, 2026
View reviewed changes
Comment thread sqlmodel/_compat.py
"type arguments. Exactly two required (e.g., dict[str, "
"Model])"
)
use_annotation = args[1]

apiwat-chantawibul Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@nkrishnaswami Should we add:

else:
    raise ValueError(f'Unsupported type {annotation} in relationship field {name}')

to avoid going into infinite recursion and get a better error message?

Could be on a separate pull request too.

apiwat-chantawibul mentioned this pull request Jun 2, 2026
Open

This branch has not been deployed

No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

YuriiMotov YuriiMotov approved these changes
+2 more reviewers
apiwat-chantawibul apiwat-chantawibul left review comments
saky-semicolon saky-semicolon left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL