Conditionally bulk_insert Messages#205
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduce conditional bulk insertion in save_django_messages by checking the DB’s can_return_rows_from_bulk_insert feature, falling back to per-instance saves when primary keys cannot be returned. Sequence diagram for conditional bulk insert in save_django_messagessequenceDiagram
participant save_django_messages
participant DB_Feature_Check
participant DjangoMessage
participant Database
save_django_messages->>DB_Feature_Check: Check can_return_rows_from_bulk_insert
alt Bulk insert supported
save_django_messages->>DjangoMessage: bulk_create(messages)
DjangoMessage->>Database: Bulk insert
Database-->>DjangoMessage: Return created messages with IDs
else Bulk insert not supported
save_django_messages->>DjangoMessage: Create message instances
loop For each message
DjangoMessage->>Database: save()
Database-->>DjangoMessage: Return created message with ID
end
end
Class diagram for updated save_django_messages logicclassDiagram
class DjangoMessage {
+thread
+message
+id
+save()
}
class save_django_messages {
+messages: list[BaseMessage]
+thread: Thread
+return: list[DjangoMessage]
}
save_django_messages --> DjangoMessage : creates
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
fjsj
left a comment
There was a problem hiding this comment.
Thanks, this looks great. Could you please add a unit test? You can mock .features.can_return_rows_from_bulk_insert if necessary, Let me know if you need assistance.
|
The tests already fail like this (repeated 21 times):
Where would the new test go? |
|
You can make a new test file under Regarding the failing tests, we'll verify. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #205 +/- ##
===========================================
+ Coverage 93.82% 100.00% +6.17%
===========================================
Files 19 5 -14
Lines 696 113 -583
Branches 47 6 -41
===========================================
- Hits 653 113 -540
+ Misses 33 0 -33
+ Partials 10 0 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I've moved over this change to this PR #211 and added the tests. The original commit has been preserved there https://github.com/vintasoftware/django-ai-assistant/pull/211/commits. Closing this one. |
Compatibility with e.g. Mysql, where bulk insert does not return generated primary keys.
Summary by Sourcery
Support databases without bulk insert row-returning capability by falling back to individual saves for message creation.
Bug Fixes:
Enhancements: