Merge lp:~rvb/maas/bug-1070765-hostname2 into lp:~maas-committers/maas/trunk
Proposed by
Raphaël Badin
Status: | Merged |
---|---|
Approved by: | Raphaël Badin |
Approved revision: | no longer in the source branch. |
Merged at revision: | 1330 |
Proposed branch: | lp:~rvb/maas/bug-1070765-hostname2 |
Merge into: | lp:~maas-committers/maas/trunk |
Prerequisite: | lp:~rvb/maas/hostname-nodegroup-unique2 |
Diff against target: |
244 lines (+91/-47) 4 files modified
src/maasserver/forms.py (+1/-1) src/maasserver/models/node.py (+37/-10) src/maasserver/tests/test_api.py (+6/-3) src/maasserver/tests/test_node.py (+47/-33) |
To merge this branch: | bzr merge lp:~rvb/maas/bug-1070765-hostname2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Review via email: mp+132569@code.launchpad.net |
Commit message
Generate a random hostname instead of creating MAC-based hostnames.
To post a comment you must log in.
Looks good!
[1]
+# Non-ambiguous characters (i.e. without 'ilouvz1250').
"s" too.
[2]
+ try:
+ self.save()
+ break
+ except ValidationError:
+ pass
There's not much to it here, but I think this is better expressed as:
try:
self. save()
pass
break
except ValidationError:
else:
i.e. we don't need to catch ValidationError from the "break"
statement.
[3]
+ def test_generate_ hostname_ does_not_ contain_ ambiguous_ chars(self) :
+ ambiguous_chars = 'ilouvz1250'
"s" here too.
[4]
+ def side_effect(*args): node.hostname hostname' , Mock(side_ effect= side_effect) )
+ def second_call(*args):
+ return 'new_hostname'
+ mock.side_effect = second_call
+ return existing_
+
+ mock = self.patch(
+ node_module, 'generate_
I think this could be simplified to not use Mock:
hostnames = [existing_ node.hostname, "new_hostname"]
node_ module, "generate_ hostname" ,
self.patch(
lambda size: hostnames.pop(0))
[5]
I was concerned that the host name could not start with a number, but
it seems that it's fine, 10gen.com for example. It might be worth
mentioning that this has been considered in a comment or docstring.
http:// en.wikipedia. org/wiki/ Hostname# Restrictions_ on_valid_ host_names