Nir Soffer has posted comments on this change. Change subject: storage: introducing vdsm-dump-chains script (part of vdsm-tool) ......................................................................
Patch Set 12: (5 comments) https://gerrit.ovirt.org/#/c/38281/12/debian/vdsm-python.install File debian/vdsm-python.install: Line 37: ./usr/lib/python2.7/dist-packages/vdsm/tool/upgrade.py Line 38: ./usr/lib/python2.7/dist-packages/vdsm/tool/upgrade_300_networks.py Line 39: ./usr/lib/python2.7/dist-packages/vdsm/tool/validate_ovirt_certs.py Line 40: ./usr/lib/python2.7/dist-packages/vdsm/tool/vdsm-id.py Line 41: ./usr/lib/python2.7/dist-packages/vdsm/tool/dump_volume_chains.py Please keep this sorted, move bellow dump_bonding_defaults.py Line 42: ./usr/lib/python2.7/dist-packages/vdsm/utils.py Line 43: ./usr/lib/python2.7/dist-packages/vdsm/vdscli.py Line 44: ./usr/lib/python2.7/dist-packages/vdsm/virtsparsify.py https://gerrit.ovirt.org/#/c/38281/12/lib/vdsm/tool/Makefile.am File lib/vdsm/tool/Makefile.am: Line 51: unified_persistence.py \ Line 52: upgrade.py \ Line 53: upgrade_300_networks.py \ Line 54: vdsm-id.py \ Line 55: dump_volume_chains.py \ This should be sorted - move below dump_bonding_defaults.py Line 56: $(NULL) Line 57: Line 58: CLEANFILES = \ Line 59: config.log \ https://gerrit.ovirt.org/#/c/38281/12/lib/vdsm/tool/dump_volume_chains.py File lib/vdsm/tool/dump_volume_chains.py: Line 52: pass Line 53: Line 54: Line 55: class ChainError(DumpChainsError): Line 56: _chainError_message = """Error in volume chain %s: %s Why _chainError_message? this is part of ChainError - there is not reason to call use a prefix. If you don't like to call it message, call it "format" or "template" Line 57: volumes and parents: %s""" Line 58: Line 59: def __init__(self, volumes_children, img_uuid): Line 60: self.volumes_children = volumes_children Line 60: self.volumes_children = volumes_children Line 61: self.img_uuid = img_uuid Line 62: Line 63: def __str__(self): Line 64: volumes = ' '.join(['%s<-%s' % (parent, child) for parent, child in This is impossible to read: - We need spaces around the "<-" - We cannot print multiple uuid pairs on the same line This must be formatted one pair per line, and the indentation must be kept on the next lines. This proves that the formatting does not belong to this error, but to the code formatting the output. Line 65: self.volumes_children]) Line 66: return ChainError._chainError_message % ( Line 67: self.img_uuid, self.__class__.__name__, volumes) Line 68: Line 62: Line 63: def __str__(self): Line 64: volumes = ' '.join(['%s<-%s' % (parent, child) for parent, child in Line 65: self.volumes_children]) Line 66: return ChainError._chainError_message % ( Use self instead of ChainError. Line 67: self.img_uuid, self.__class__.__name__, volumes) Line 68: Line 69: Line 70: class DuplicateParentError(ChainError): -- To view, visit https://gerrit.ovirt.org/38281 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I428c443bb7d6b2a504a6f77efcd4838f7ae6c404 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Darshan N <dnara...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Vladik Romanovsky <vladik.romanov...@gmail.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yaniv Dary <yd...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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