Opened 6 years ago

Closed 6 years ago

#15647 closed (fixed)

in_bulk() should not type check its input

Reported by: Calvin Spealman Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

#12251 added additional types it will allow, but why does it typecheck at all? For example, I wanted to pass it a ValuesList, which had come from queryset.values_list('id', flat=True), but I don't just want another type added to the assert. Can the assert be removed entirely? it should work with any iterable I pass it.

Attachments (1)

15647-in-bulk-accepts-iterables.patch (818 bytes) - added by Calvin Spealman 6 years ago.
Removes assert from in_bulk and adds tests for iterables passed to it

Download all attachments as: .zip

Change History (4)

comment:1 Changed 6 years ago by Adrian Holovaty

Needs tests: set
Triage Stage: UnreviewedAccepted

Good question. I don't recall why we added that typechecking, but it seems like we could just remove it. Can you (or somebody) write some tests that allow the passing of any iterable, like a ValuesList?

Changed 6 years ago by Calvin Spealman

Removes assert from in_bulk and adds tests for iterables passed to it

comment:2 Changed 6 years ago by Calvin Spealman

Has patch: set

I made the tests against a more generic iterable, falling in line with the reasoning that the problem is type-specific logic so I didn't want to test just one more type on it.

comment:3 Changed 6 years ago by Adrian Holovaty

Resolution: fixed
Status: newclosed

In [15922]:

Fixed #15647 -- Changed in_bulk() not to type check its input, which now allows for passing any iterable. Thanks, calvinspealman

Note: See TracTickets for help on using tickets.
Back to Top