mooli tayer has posted comments on this change. Change subject: Adding remove/disable verbs to vdsm-tool for admin usages ......................................................................
Patch Set 4: (4 comments) http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/constants.py.in File lib/vdsm/constants.py.in: Line 43: Line 44: # Line 45: # SASL definitions Line 46: # Line 47: P_EXEC_SASL = '/usr/sbin/saslpasswd2' > I think it should be similar to other EXT_GREP = '@GREP_PATH@', and determi Done Line 48: SASL_USERNAME = "vdsm@ovirt" Line 49: Line 50: # This is the domain version translation list Line 51: # DO NOT CHANGE OLD VALUES ONLY APPEND http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 354: for c in __configurers: Line 355: if c.getName() in args.modules: Line 356: try: Line 357: c.removeConf() Line 358: except: > please do not swallow exceptions. Done Line 359: failed.append(c.getName) Line 360: if failed: Line 361: sys.stderr.write( Line 362: "Could not remove configuration for modules %s\n" % ','.join(failed), Line 359: failed.append(c.getName) Line 360: if failed: Line 361: sys.stderr.write( Line 362: "Could not remove configuration for modules %s\n" % ','.join(failed), Line 363: ) > I think this can be dropped in favour of boolean and print above. Done Line 364: raise RuntimeError("Remove configuration failed") Line 365: Line 366: Line 367: def _parse_args(action): http://gerrit.ovirt.org/#/c/21772/4/lib/vdsm/tool/passwd.py File lib/vdsm/tool/passwd.py: Line 53: Remove vdsm password for libvirt connection Line 54: """ Line 55: rc, out, err = utils.execCmd( Line 56: (constants.P_EXEC_SASL, '-p', '-a', 'libvirt', '-d', constants.SASL_USERNAME,), Line 57: raw=True, > why raw? I saw that raw=False (default) splits output lines, and we don't need that. I not sure why its a part of the function? Remove the raw? Line 58: ) Line 59: if rc != 0: -- To view, visit http://gerrit.ovirt.org/21772 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie7f2c031436a6d202f856c24d9c9420c8bfdf6df Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer <mta...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
