Opened 16 years ago

Closed 13 years ago

#7566 closed (fixed)

dumpdata command should not use model._default_manager.all()

Reported by: anonymous Owned by: nobody
Component: Core (Serialization) Version: dev
Severity: Keywords: dumpdata
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

dumpdata command should not use model._default_manager.all() for retrieving objects because model can use custom Manager with redefined .all() method.
better way for retrieving all objects is using QuerySet(model).all()

Attachments (2)

dumpdata-fix.patch (911 bytes ) - added by anonymous 16 years ago.
6155.diff (1.5 KB ) - added by Karen Tracey <kmtracey@…> 16 years ago.
Patch that updates doc as well

Download all attachments as: .zip

Change History (7)

by anonymous, 16 years ago

Attachment: dumpdata-fix.patch added

by Karen Tracey <kmtracey@…>, 16 years ago

Attachment: 6155.diff added

Patch that updates doc as well

comment:1 by Karen Tracey <kmtracey@…>, 16 years ago

Triage Stage: UnreviewedAccepted

Seems like a good idea, since I'd expect you'd normally want to dump everything even if the default manager excludes some records. I was unaware of that syntax for QuerySet creation when the fix for #6155 went in. But if this change is made then the doc note about the default manager being used needs to be deleted, since it won't be true any more. I updated the patch to do that as well.

comment:2 by Malcolm Tredinnick, 16 years ago

Triage Stage: AcceptedDesign decision needed

I'm -1 on this change. If you have installed a default manager it's because they're the records you consider the "whole set" by default. And that's what dumpdata works with. If you wanted the whole set to be "everything", you wouldn't have overridden the default manager.

comment:3 by Karen Tracey <kmtracey@…>, 16 years ago

Eh, for my case I don't think of what the default manager returns as the "whole set". It's more the publicly-visible set. There's more in the database (which existed long before I put a Django front-end on it) than I care to expose via the website front-end, and I use excluding default managers to limit what can be seen in the normal case via the web site. That said, I've never actually used dumpdata (I backup/migrate/move the DB with database utilities directly) so I'm not particularly qualified to comment on how it should behave. But if I did need to use it, I think I'd want it to dump all the data.

in reply to:  3 comment:4 by anonymous, 16 years ago

Replying to Karen Tracey <kmtracey@gmail.com>:

Eh, for my case I don't think of what the default manager returns as the "whole set". It's more the publicly-visible set. There's more in the database (which existed long before I put a Django front-end on it) than I care to expose via the website front-end, and I use excluding default managers to limit what can be seen in the normal case via the web site. That said, I've never actually used dumpdata (I backup/migrate/move the DB with database utilities directly) so I'm not particularly qualified to comment on how it should behave. But if I did need to use it, I think I'd want it to dump all the data.

I agree with Karen Tracey. I use custom manager for creating versions of object, they makes visible only last version. If i execute dumpdata, i expect that all objects will be saved. Creating new method for filtering objects eliminates the need for a possible replace of the Manager, because that need to change all of the related sources.

comment:5 by Ramiro Morales, 13 years ago

Resolution: fixed
Status: newclosed

This was fixed in r13669 with a Solomonic decision.

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