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

Revision history for this message
Mike Pontillo (mpontillo) wrote :

This code is very difficult to review since its complexity has gone through the roof.

I think I said this the last this was changed, but I really think we should break this down into multiple queries (via views, perhaps) so that individual pieces can be tested easier.

For now, I attempted to review by comparing the added/changed tests to the added/changed code. I think I would like to see more specific tests to capture each edge case, such as:

    test__default_domain_is_extra_special_ipv4
    test__default_domain_is_extra_special_ipv6
    test__domains_are_special
    test__attached_addresses_only_map_back_to_node

That way it's clearer to future maintainers how each edge case and branch called out in the SQL maps to a requirement for a specific behavior.

review: Needs Fixing

« Back to merge proposal