Code

Opened 3 years ago

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

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

comment:2 Changed 3 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 3 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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.