Martin Peřina has posted comments on this change. Change subject: vdsm: adding handling for NGN in osinfo.py ......................................................................
Patch Set 7: (2 comments) https://gerrit.ovirt.org/#/c/57620/7/lib/vdsm/osinfo.py File lib/vdsm/osinfo.py: Line 124: Line 125: def _parse_ngn_version(path, data): Line 126: return tuple( Line 127: [ Line 128: data.get(k, '').replace('"', '') Hmm, maybe using strip('"') instead of replace is better idea, because it doesn't remove apostrophes from inside the string Line 129: for k in 'NAME', 'VERSION_ID', 'VARIANT' Line 130: ] Line 131: ) Line 132: Line 141: OS_RELEASE_FILE = '/etc/os-release' Line 142: version = release_name = '' Line 143: data = None Line 144: if os.path.exists(OS_RELEASE_FILE): Line 145: data = get_data(OS_RELEASE_FILE) Wouldn't it be more readable to do NGN check here: data = None if os.path.exists(OS_RELEASE_FILE): data = get_data(OS_RELEASE_FILE) if ( data and data.get('VARIANT_ID', '').strip('"') == 'ovirt-node' ): osname, version, release_name = _parse_ngn_version(OS_RELEASE_FILE, data) else: osname = _release_name() try: ... return dict(release=release_name, version=version, name=osname) Line 146: osname = _release_name() Line 147: try: Line 148: if osname == OSName.RHEVH or osname == OSName.OVIRT: Line 149: version, release_name = _parse_node_version('/etc/default/version') -- To view, visit https://gerrit.ovirt.org/57620 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17337643cd4e986c09b07137d80da3555161ee70 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[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
