Code review comment for lp://staging/~brendan-donegan/checkbox/bug1070497

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

54 + while (current_time - start_time).seconds < ARP_WAIT:
56 + known_ips = subprocess.check_output(["arp", "-a", "-n"])

This is wrong on two accounts:

1) It is a busy loop that spawns processes. We should really wait between iterations.
2) It a time loop using wall clock time. This makes it susceptible to various clock shift issues (for instance, ntp clock adjustment)

To fix 1 I'd add a simple delay (it's better to be event triggered but here we don't have that luxury

To fix 2 I'd convert this to an explicit loop of X attempts that break as soon as an attempt succeeds.

review: Needs Fixing

« Back to merge proposal