Artyom Lukianov has posted comments on this change. Change subject: hooks:checkips: add checkips hook ......................................................................
Patch Set 11: (4 comments) https://gerrit.ovirt.org/#/c/54102/11/vdsm_hooks/checkips/after_get_stats.py File vdsm_hooks/checkips/after_get_stats.py: Line 47: Line 48: CONNECTIVITY_TIMEOUT = 60 Line 49: Line 50: Line 51: def _is_network_accessible(net, stats_dir): > which name did you choose? I found "is_network_accessible" quite understand I thought about _is_network_file_up_to_date Line 52: file_path = os.path.join(stats_dir, net) Line 53: if os.path.exists(file_path): Line 54: return ( Line 55: time.time() - os.stat(file_path).st_mtime <= https://gerrit.ovirt.org/#/c/54102/11/vdsm_hooks/checkips/checkipsd File vdsm_hooks/checkips/checkipsd: Line 35: CHECKIPV6 = 'checkipv6' Line 36: VDSM_CHECKIPS = 'vdsm-checkips' Line 37: Line 38: Line 39: def get_ping_addresses(net_attrs): > But why not add it to PYTHONPATH? you can do it within the script, with sys Done Line 40: ping_addresses = [] Line 41: if 'custom' in net_attrs: Line 42: for address_type in (CHECKIPV4, CHECKIPV6): Line 43: if address_type in net_attrs['custom']: Line 54: with open(file_path, 'a'): Line 55: os.utime(file_path, None) Line 56: Line 57: Line 58: def _ping_address(address, address_type, net): > this means you have a serious issue for non-VM networks, where the name of mm I believe it just differences in terminology, because when I hear word interface I think about some host physical interface, so I can change it to 'interface' if it makes the variable name more understandable Line 59: ping_cmd = 'ping' if address_type == CHECKIPV4 else 'ping6' Line 60: command = [ Line 61: ping_cmd, Line 62: '-c', '1', PS11, Line 64: -I', net, > actually, we should not add the explicit -I argument. The user should choos Done -- To view, visit https://gerrit.ovirt.org/54102 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53cec37310f0f1844d6fe244419fd8c10e9b7ebb Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Artyom Lukianov <[email protected]> Gerrit-Reviewer: Artyom Lukianov <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Edward Haas <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Yaniv Kaul <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
