Am 11.11.2009 um 17:50 schrieb Michael Glassford:
>
> Johannes Dollinger wrote:
>>
>> Am 10.11.2009 um 17:22 schrieb Michael Glassford:
>>
>>> I haven't had a chance to look at the patch yet, but what you
>>> describe
>>> here sounds good. I don't have any problem with you "hijacking" the
>>> work.
>>>
>>> Did your patch deal at all with the unit tests in my patch that
>>> intentionally failed to expose things that weren't working right
>>> yet?
>>
>> Only the first of your patches contains tests.
>
> Oops, right you are. The unit tests were unintentionally omitted. I've
> uploaded a new patch which is the pretty much the same as my previous
> patch but with my unit tests included (although comparing the two
> now I
> see a few other differences, probably because I also updated to the
> latest code in SVN).
>
>> But the ON DELETE
>> related tests in this patch that do not test database-level support
>> should be covered by my tests in modeltests/on_delete/.
>
>> Which of your tests failed?
>
> The tests that fail are in tests/modeltests/on_delete_django/tests.py.
> They have names like test_issue_1a, test_issue_1b, test_issue_2a,
> test_issue_2b, etc. The comments on the tests indicate the expected
> result. Once the issues they test for are fixed, they should all pass.
Thanks a lot. I ran your tests with my patch. There were a few
(harmless) failing tests:
- sanity checks for SET_NULL on fields that are not nullable or
SET_DEFAULT on fields that have no default value. I moved the code to
model validation (and added tests there), so I just removed these tests.
- you assumed SET_NULL as the default behavior for nullable FKs. That
would be backwards incompatible. I changed the default to CASCADE, so
I had to update related tests.
The remaining failing tests are test_issue_2a, -_2b, -_2d, -_2e. They
test if related object caches are updated/cleared after `delete()`,
e.g.:
data = Data.objects.create(data=1)
m = M.objects.create(fk=data) # fk is a nullable ForeignKey
data.delete()
self.assertEqual(None, m.fk) #Fails
I don't think this is required. #17[1] was just rejected in the 1.2
voting process, and without identity mapping (or a global object
collection or excessive use of signals) you would not be able to cover
all use-cases. If you called Data.objects.all().delete() instead of
data.delete() you'd still have outdated related object caches. Or if
you had obtained m through M.objects.get(..). Or ..
I'm aware that django currently *does* null out PKs of deleted objects
as well as nullable references to those objects (which will only be
present in objects that have themselves been deleted). But it's
limited to instances reachable from Model.delete() via reverse one-to-
one descriptors and instances passed to signal handlers.
And finally: it's neither documented nor tested (you can remove the
relavant four lines from delete_objects() without breaking existing
tests).
My patch actually changes the behavior of Model.delete() in this
regard: reverse one-to-one caches are not updated.
I'd rather see model instances as a "checkout" from the db that can
get out of sync. Are there any deeper problems with this approach?
[1] http://code.djangoproject.com/ticket/17
__
Johannes
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=.
