Looks good. As discussed on IRC, it means people with existing nodes with invalid labels will need to rename their nodes before they can do any modifications to them but:
a) the problem will be quite easy to identify (the validation error will be pretty clear)
b) I don't expect many people will have to face this problem
[0]
> If the hostname is not valid according to RFCs 952 and 1123.
I know a hostname is a hostname (as in "A Big Mac is a Big Mac") but I think this is worth repeating in a place that makes it to the documentation, i.e. in ":ivar hostname: This `Node`'s hostname." in src/maasserver/models/node.py:Node
[1]
41 + if len(label) > 63:
42 + raise ValidationError("Label is too long: '%s'." % label)
The error would be more informative if it mentioned the limit.
[2]
40 + raise ValidationError("Hostname contains empty label.")
41 + if len(label) > 63:
42 + raise ValidationError("Label is too long: '%s'." % label)
Do you think it's obvious to people what a "label", in this context, means? I must admit it was not obvious to me before I read your comments. Maybe "part"?
[3]
44 + raise ValidationError(
45 + "Label cannot start or end with hyphen: '%s'." % label)
You can use %r here, this saves you from having to include the single quotes around the string.
Looks good. As discussed on IRC, it means people with existing nodes with invalid labels will need to rename their nodes before they can do any modifications to them but:
a) the problem will be quite easy to identify (the validation error will be pretty clear)
b) I don't expect many people will have to face this problem
[0]
> If the hostname is not valid according to RFCs 952 and 1123.
I know a hostname is a hostname (as in "A Big Mac is a Big Mac") but I think this is worth repeating in a place that makes it to the documentation, i.e. in ":ivar hostname: This `Node`'s hostname." in src/maasserver/ models/ node.py: Node
[1]
41 + if len(label) > 63: ("Label is too long: '%s'." % label)
42 + raise ValidationError
The error would be more informative if it mentioned the limit.
[2]
40 + raise ValidationError ("Hostname contains empty label.") ("Label is too long: '%s'." % label)
41 + if len(label) > 63:
42 + raise ValidationError
Do you think it's obvious to people what a "label", in this context, means? I must admit it was not obvious to me before I read your comments. Maybe "part"?
[3]
44 + raise ValidationError(
45 + "Label cannot start or end with hyphen: '%s'." % label)
You can use %r here, this saves you from having to include the single quotes around the string.
[4]
59 + hostname = CharField( [validate_ hostname] )
60 + max_length=255, default='', blank=True, unique=True,
61 + validators=
Once more, maybe mention the RFCs in the help_text.