Code review comment for lp:~julian-edwards/maas/models-for-static-dhcp

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Please document IPAddress. It's really a static IP address, not just any dynamic-or-static IP address, right?

Why the foreign key from IPAddress to MACAddress in the first place? Wasn't that going to be a many-to-many relationship? Or is this transitional?

I'm not holding this up with my vote — whatever comes out, no harm done at this stage. Better to land and improve than to fork and languish. But to me, the "type" field affects the behaviour of the foreign key in too profound a way.

The biggest difference I see between the four types of relationship is what happens when a node is deallocated. If an IPAddress is "auto" or "extra," it goes down with the ship. If it's "unmanaged" or "sticky," it survives. I think those are different structural relationships, so maybe they should be two alternative foreign keys, with different cascading behaviours.

Of course my "biggest difference" only gets us to 2 types, out of 4. What are the other differences? It seems to me that:

 * "Auto" is like "extra" except it can't be deallocated separately. Could be a separate relationship, or just a boolean attribute.

 * "Unmanaged" is like "sticky" except the machine on the other end is not a node. But does that matter beyond creation and display? If you set an IP address for a machine's MAC address, then registered that machine as a node, ISTM the IP address would implicitly go from "unmanaged" to "sticky" without affecting the world in any way.

There may also be cardinality differences. I can't imagine attaching two MACs to the same "auto" address, but it might make sense for "extra" addresses. Splitting out the relationships may make the rules clearer, and easier to enforce as constraints.

review: Approve

« Back to merge proposal