Nir Soffer has posted comments on this change. Change subject: StorageDomainManifest: introduce class heirarchy ......................................................................
Patch Set 9: (6 comments) https://gerrit.ovirt.org/#/c/41993/9/tests/manifestTests.py File tests/manifestTests.py: Line 62: open(self.manifest.metafile, "w").truncate(self.MDSIZE) Line 63: self.manifest.bind( Line 64: fileSD.FileSDMetadata(self.manifest.metafile)) Line 65: except: Line 66: shutil.rmtree(self.mountpoint) You are hiding errors in setup - you must re-raise. Line 67: Line 68: def tearDown(self): Line 69: shutil.rmtree(self.mountpoint) Line 70: Line 81: self.manifest.bind(BlockSDMetadata()) Line 82: self.lvm = FakeLVM(self.mountpoint) Line 83: self.lvm.createVG(sdUUID) Line 84: except: Line 85: shutil.rmtree(self.mountpoint) Same issue - re-raise Line 86: Line 87: def tearDown(self): Line 88: shutil.rmtree(self.mountpoint) Line 89: https://gerrit.ovirt.org/#/c/41993/9/tests/storagefakelib.py File tests/storagefakelib.py: Line 30: def createVG(self, vgName, *args): Line 31: try: Line 32: os.mkdir(os.path.join(self.root, 'dev', vgName)) Line 33: except: Line 34: raise se.VolumeGroupCreateError(vgName, []) Not sure this error is useful - if mkdir fails in the test, I want to see the original exception. When we want to simulate errors, we want to control which error will be raised. Line 35: Line 36: def createLV(self, vgName, lvName, size, *args): Line 37: lvFile = os.path.join(self.root, 'dev', vgName, lvName) Line 38: try: Line 33: except: Line 34: raise se.VolumeGroupCreateError(vgName, []) Line 35: Line 36: def createLV(self, vgName, lvName, size, *args): Line 37: lvFile = os.path.join(self.root, 'dev', vgName, lvName) Use self.lvPath(vgName, lvName) Line 38: try: Line 39: open(lvFile, "w").truncate(size << 20) Line 40: except: Line 41: raise se.CannotCreateLogicalVolume(vgName, lvName) Line 35: Line 36: def createLV(self, vgName, lvName, size, *args): Line 37: lvFile = os.path.join(self.root, 'dev', vgName, lvName) Line 38: try: Line 39: open(lvFile, "w").truncate(size << 20) Use with. Line 40: except: Line 41: raise se.CannotCreateLogicalVolume(vgName, lvName) Line 42: Line 43: def lvPath(self, vgName, lvName): Line 37: lvFile = os.path.join(self.root, 'dev', vgName, lvName) Line 38: try: Line 39: open(lvFile, "w").truncate(size << 20) Line 40: except: Line 41: raise se.CannotCreateLogicalVolume(vgName, lvName) Same Line 42: Line 43: def lvPath(self, vgName, lvName): -- To view, visit https://gerrit.ovirt.org/41993 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia275424a8d423722efb229a4d8545f035fe5fa51 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
