Code review comment for lp:~lamont/maas/bug-1660188

Revision history for this message
Gavin Panella (allenap) wrote :

Just a few comments from quickly going over this code.

You were worried about query counts going up. That's a good thing to have noticed and to worry about, but it's nothing unusual: it's the curse of ORMs. The dehydrate_* method pattern is nice to work with but it exacerbates this problem.

The first tool I'd reach for is tests, specifically using CountQueries / count_queries. Typically I'd do something like "render n things" and then "render n+1 things" and assert that the query count is the same for both. Then I'd add pre-fetching in the "right" places until the test passes.

Overall, there's a lot of stuff in here. It's already at ~1000 lines of diff, but it's densely populated. Splitting it into 2 or more pieces will help a lot at review time.

« Back to merge proposal