On Thu, May 30, 2013 at 11:22:53AM +0100, Tiago Gomes wrote:
+ '--nic1', 'hostonly',
'--hostonlyadapter1', hostonly_iface,
+ '--nic2', 'nat', '--natnet2', 'default'],
This hard-codes the VirtualBox network name natnet2. Is that network always there,
and always the one we should be using? Or should we create that too, if it's missing?
+ if "eth0:" not in network_config:
+ raise cliapp.AppException('NETWORK_CONFIG does not contain ' + \
+ 'the eth0 configuration')
Minor Python formatting nitpick: the above would be better formatted as:
if "eth0:" not in network_config:
raise cliapp.AppException(
'NETWORK_CONFIG does not contain the eth0 '
'configuration')
Python string literals have implicit concatenation, and breaking the
lines as above is easier to read, I find, since it allows more text
per line of source code.
In any case, the backslash is not necessary for line continuation,
since Python doesn't mind expressions within parentheses to be
continued to the next line.
+ iface = cliapp.runcmd(
+ ['ssh', ssh_host, 'VBoxManage', 'list',
'hostonlyifs'],
+ ['grep', '-B', '3', host_ipaddr],
+ ['head', '-n', '1'],
+ ['sed', 's/.* //'],
+ ['tr', '-d', '\n'])
I'd prefer this kind of thing to be running only one external command
(in this case, with cliapp.ssh_runcmd), and then do the string parsing
in Python. But if this works, it's OK to leave it as is. However, a
comment explaining what is happening, and maybe what the expected
output for "VBoxManage list hostonlyifs" is would be good, for the sake
of the next person to modify the code (which might happen after VBoxManage
changes it output format).
--
http://www.codethink.co.uk/ http://wiki.baserock.org/ http://www.baserock.com/