Opened 4 years ago

Closed 4 years ago

#15647 closed (fixed)

in_bulk() should not type check its input

Reported by: calvinspealman 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 calvinspealman 4 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 4 years ago by adrian

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 4 years ago by calvinspealman

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

comment:2 Changed 4 years ago by calvinspealman

  • 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 4 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

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