Opened 13 years ago

Closed 13 years ago

#15696 closed New feature (wontfix)

Add pre_insert signal to loaddata command

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

Description

Here's a patch that adds a pre_insert signal to the loaddata command, allowing to do extra modifications on deserialized objects before saving them to the database.

Here's my use case: SQL databases use integer primary keys, MongoDB uses ObjectIds, and I want to re-use SQL-style fixtures (as found in the Django test suite) to test Django MongoDB Engine (the Django MongoDB backend). I think this is a painless way to solve this problem: Very little code changes and doesn't require any ugly hacks.

Would be great if this could be applied!

Attachments (2)

serializer-pre-insert-signal.patch (1.4 KB ) - added by Jonas H. 13 years ago.
with-tests.patch (2.7 KB ) - added by Jonas H. 13 years ago.
Fixed version with tests

Download all attachments as: .zip

Change History (12)

by Jonas H., 13 years ago

comment:1 by Jonas H., 13 years ago

Version: 1.2SVN

comment:2 by Luke Plant, 13 years ago

Type: New feature

comment:3 by Luke Plant, 13 years ago

Severity: Normal

comment:4 by Jonas H., 13 years ago

Any way to get this into trunk soon?

comment:5 by Julien Phalip, 13 years ago

Has patch: set
Needs documentation: set
Needs tests: set
Triage Stage: UnreviewedAccepted

This sounds like a useful feature, but to stand any chance of inclusion the patch would need some tests and documentation.

by Jonas H., 13 years ago

Attachment: with-tests.patch added

Fixed version with tests

comment:6 by Jonas H., 13 years ago

Needs tests: unset

Where should the documentation go to? The 'loaddata' section in the commands reference? The testing docs?

comment:7 by Russell Keith-Magee, 13 years ago

Patch needs improvement: set

I disagree that a signal is the right way to handle this. Signals, even when unconnected, have a cost, and this would impose a cost on every fixture load (and, by extension, every test in every test run) in order to support what seems to be to be an extreme edge case requirement.

My gut tells me that a better approach would be a custom serializer. That would isolate the extra cost to the specific deserializer that needs the extra data conversion features.

Answering your question about tests: the tests should go in the regressiontests/fixtures_regress test suite; the loaddata command is tested there as a side effect of testing fixtures themselves.

comment:8 by Russell Keith-Magee, 13 years ago

Triage Stage: AcceptedDesign decision needed

comment:9 by Jonas H., 13 years ago

Russell, that's true, although I think that's negligible - running all model tests, I get a difference of half a second (48.3 vs. 47.9 seconds), for Django's ~3500 tests that's an addition of about 3 seconds.

(btw, my question was about documentation, not tests.)

comment:10 by Jacob, 13 years ago

Easy pickings: unset
Resolution: wontfix
Status: newclosed

I find myself agreeing with Russ that a custom serializer is a better approach here. I just can't find a compelling use case for this signal beyond the very specific one you've posted. Wary of feature creep, I'm marking this wontfix.

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