Code review comment for lp:~julian-edwards/maas/acl-for-dns-trigger-rewrite

Thanks for the review!

On Monday 01 Sep 2014 03:20:57 you wrote:
> Review: Approve
>
> It looks to me as if test_writes_dns_when_network_edited should be called
> test_writes_dns_when_network_created. Why does it go through the form?

1. It's not just creating, it's any editing (which includes creating).
2. Using the form just covers actual code use style. I didn't want to rely on
any factory side effects.

> I still don't like signals for this kind of thing. Do we know what we'll be
> doing during network auto-discovery? Will we implicitly be firing off a
> bunch of DNS server restarts concurrently, which may then get confused and
> break? It would make more sense to me to set some kind of “dirty” flag on
> DNS, and process that separately.

I hate signals too, but there's not much choice. I think we've covered this
to death already.

Given that this will condense into an RPC job at some point, the easiest (and
I'd argue *right) place to coalesce jobs is right there. For now I figured
the rate at which Networks are changed will be absolutely tiny.

> Also, consider triggering the signal only on specific field changes.
> Updating the description on a network, or saving it without changes, or
> perhaps updating some future field that may see routine changes, probably
> needn't activate the whole DNS machinery.

Again, the number of changes to Networks will be tiny, it's not worth it IMO,
especially as we add more fields that *will* require a trigger would
potentially get missed.

cheers.