DjangoBot: #django-dev

I don't suppose anyone recalls which ticket tracked the undoing of tag library module name mangling..

mattmcc: you mean #6587?

Sweet, thanks.

Alex_Gaynor: ping

I'm seriously going to go blank and lock the damn "threadlocal user" wiki page.

Any objections? ;)

ubernostrum: +1

ubernostrum: oh noes ;) what's happening?

if anyone agrees with the last comment on #8960, I'd happily supply a patch. the current patches seems a bit ugly to me... This is 'view on site' breakage if contrib.sites is not installed. Current patches start introspecting the current model to see if there's a related field to sites. I like the RequestSite fallback idea much better.

+1

mitsuhiko: pong

Alex_Gaynor: heyho :)

ubernostrum: ping

freakboy3742: if you have a moment sometime, i'd welcome thoughts on the approach in #12953

carljm: looking

is #5986 believed to be a dupe of #8164? Since 5986 related to (non-model) Forms, and was closed as a dupe of 8164 which was fixed for only ModelForm

carljm: On the surface, it looks ok

I'm at work at the moment, so I can't do a complete teardown

but I'll put it on my list

On the whole, the approach looks good - yet another score for proper models for m2m

:)

THere's one edge case that I don't know if you're testing for

carljm: given any thought to my idea to improve efficiency of deletions of the object graph?

a manually created m2m intermediate that has a FK or m2m of it's own

I haven't worked through the logic, but my but tells me there might be an edge case in there that is, at the very least, worth checking for.

freakboy3742: I think that case should fall out automatically, given that I call _collect_sub_objects on the through model instances

carljm: test it!

but I'll check and make sure it's tested somewhere

There's an off chance it may already be tested

i think this improves the code maintainability; i guess my one question is how much effort i need to put into tracking down and quantifying possible speed implications

or is this a case of "do the right thing and if it turns out we can speed it up later, we will"

carljm: well we know we can speed it up later

I already presented my architecture for doing so

yeah, i think we can too. but not before 1.2

agreed

the other problem with your (otherwise excellent) proposal for speeding it up is that it pretty much trashes the fix we already put in for #6191

at least i can't figure out how to make them compatible

because the admin needs a per-object hierarchy

(or at least Adrian said that objects-to-be-deleted display has to stay hierarchical)

carljm: Make it work, then make it work fast :-)

carljm: well we can maintain the per object traversal for the obj.delete() case

Given an abstract graph of the obj relations translating it into per-object traversal or bulk traversals shouldn't be overly difficult

freakboy3742: great. as long as speed won't keep this fix from getting in for 1.2, we'll work on a post-1.2 fix for speeding up _collect_sub_objects

Alex_Gaynor: the admin also does QuerySet.delete(), if you delete using the bulk action

but I think you're right, the translation from model-graph to object-graph should be possible, just perhaps not fast

carljm: well we can do invidiual _collect_sub_objects to display the graph, and then use the optimized calll for the deletion itself?

but nobody cares too much if the admin delete-confirm page is slow

carljm: we can also cache the Delete graph ones its generated

the thing I want to avoid is getting back to a situation where its one code path for the confirmation display, and a different one for the deletion

but if we do it in terms of generating model-graph + pk-lists (the fast way) in either case, then an optional step of translating that into an object-graph, i think that would be reasonable

carljm: it won't be, there's be a DeleteGraph class that provides the data, and then Deleters that take a graph, and either a qs or an indiivdual obj and traverse the graph and do deletions, one Deleter will be efficiently implemented for QS with multiple items, and one will be for single objects

even if the second optional step is slow

The DeleteGraph is the part where the logic is tricky

finding objects given a DeleteGraph isn't difficult

yep, that all makes sense. as you say we can cache the DeleteGraph on the model class, though that's not the slow part (doesn't even hit the DB)

actually though, that doesn't really address the multiple-code-paths concern

carljm: It'd deifnitely help, especially since we can cache it for foverver

it's still entirely possible for one Deleter to have a bug the other one doesn't

carljm: I tihnk the hard to get write codepath is the DeleteGraph generation, not the traverseing

for them to come up with different sets of objects

carljm: thats true, but I don't see a way around it

carljm: Actually... we can just make obj.delete() -> Model.objects.fitler(pk=obj.pk).delete()

TADA single path

carljm: although i don't see how that helps us for showing the deletion tree

the multiple code paths i'm concerned about aren't single object vs queryset

personally i think the fast approach will be better for general use in either case

once you're at depth two it's multiple objects either way

carljm: I'm saying we can fix taht for the actual deletion, but that avoids generating the tree, which is whate the admin needs

yeah

carljm: so yeah, I don't see a way to avoid some amount of duplication

it's the admin tree i'm concerned about. i want that list of objects generated in a way that makes it trivially obvious that it will contain the same set of objects as the ones that are actually deleted

which I think is doable, but not quite using the DeleteGraph/Deleter architecture

if we can produce a DeleteGraph with lists of PKs for each model involved

carljm: Actually, I guess given a set of objects that the deleter has found (efficiently) it sholud be possible to regenerate a tree from them

using the fast route

exactly

then we generate an object tree based on that

GMTA

heh

sounds good

that whole process is then slower than what we do now

but who cares, its just for the confirmation page

right, bulk deletion will be hugely faster

and worth it for the reliability IMO

agreed

alright then! plan is in place. now who will get around to turning it into code first...

have you filed a ticket for it yet?

no

i'm on it

thanks