Conversation
aminalaee
left a comment
aminalaee
left a comment
There was a problem hiding this comment.
Thanks for the PR. I had a look and could see a few points.
|
I have to admit I'm not sure how the API should look like. I'll probably take a look at a few examples in other projects. Maybe you can update the PR to show the current signature of the bulk update. |
|
https://github.com/collerek/ormar/blob/1ffb28d7b06f3f7989a6219c900e2fd441cbea42/ormar/queryset/queryset.py#L1064 I think that ormar do the same thing here. |
|
It is also standard bulk update as Django: https://docs.djangoproject.com/en/4.0/ref/models/querysets/#bulk-update |
orm/models.py
Outdated
| ] | ||
| expr = ( | ||
| self.table.update() | ||
| .where(self.table.c.id == sqlalchemy.bindparam(self.pkname)) |
There was a problem hiding this comment.
Need to get primary key column dynamically. The id is hard-coded.
orm/models.py
Outdated
| "lte": "__le__", | ||
| } | ||
|
|
||
| MODEL = typing.TypeVar("MODEL", bound="Model") |
There was a problem hiding this comment.
What value does this bring? I mean we could call bulk_update with Model itself. right?
There was a problem hiding this comment.
What do you mean? We use it as a type annotation for obj
There was a problem hiding this comment.
Yeah I understand. I mean you've done this:
def bulk_update(self, objects: typing.List[MODEL], ...):
....What would be the difference if we did:
def bulk_update(self, objects: typing.List[Model], ...):
....There was a problem hiding this comment.
In this case Model will be undefined because it has been defined after bulk_update
|
|
||
| await self.database.execute(expr) | ||
|
|
||
| async def bulk_update( |
There was a problem hiding this comment.
Sorry I should've noticed this earlier, apologies for that.
But maybe a general refactor would be useful here?
There's a lot of nested code here and it's not very readable. What do you think?
There was a problem hiding this comment.
Yeah I agree with you it needs to be more readable
Mng-dev-ai
force-pushed
the
add-support-for-bulk-update
branch
from
9f4e83d to
daf5b5e
Compare
Mng-dev-ai
force-pushed
the
add-support-for-bulk-update
branch
from
daf5b5e to
6f79635
Compare
|
TBH I think this is really good and useful but as long as the We can keep it until then. |
|
So, maybe we need to add it to |
|
yes, I have an old PR which is doing it but It's not really the best way but it can give you some ideas. Feel free to take a look at that and start your own. |
|
Yeah sure, I will take a look at it. |
* The bulk_update is an adaptation of encode/orm#148
2 participants
No description provided.