Change in vdsm[master]: StorageDomainManifest: introduce class heirarchy

nsoffer at redhat.com nsoffer at redhat.com
Wed Jun 17 18:08:37 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: StorageDomainManifest: introduce class heirarchy
......................................................................


Patch Set 12: Code-Review+1

(6 comments)

https://gerrit.ovirt.org/#/c/41993/12/tests/manifest_tests.py
File tests/manifest_tests.py:

Line 32: class FileManifestTests(VdsmTestCase):
Line 33:     MDSIZE = 524288
Line 34: 
Line 35:     def test_getreaddelay(self):
Line 36:         with namedTemporaryDir() as path:
I would call path tmpdir to avoid confusion.
Line 37:             manifest = self.make_manifest(path)
Line 38:             self.assertIsInstance(manifest.getReadDelay(), float)
Line 39: 
Line 40:     def make_manifest(self, path):


Line 49:         manifest.bind(dict())
Line 50:         return manifest
Line 51: 
Line 52: 
Line 53: class BlockManifestTests(VdsmTestCase):
lets add empty line after the class for consistency.
Line 54:     def test_getreaddelay(self):
Line 55:         with namedTemporaryDir() as path:
Line 56:             manifest = self.make_manifest(path)
Line 57:             vg = manifest.sdUUID


Line 53: class BlockManifestTests(VdsmTestCase):
Line 54:     def test_getreaddelay(self):
Line 55:         with namedTemporaryDir() as path:
Line 56:             manifest = self.make_manifest(path)
Line 57:             vg = manifest.sdUUID
better call this vg_name, to avoid confusion later with vg namedtuple returned from lvm apis.
Line 58:             lvm = FakeLVM(path)
Line 59:             make_metafile(lvm.lvPath(vg, 'metadata'))
Line 60: 
Line 61:             with MonkeyPatchScope([(blockSD, 'lvm', lvm)]):


Line 64:     def make_manifest(self, path):
Line 65:         sduuid = str(uuid.uuid4())
Line 66:         manifest = blockSD.BlockStorageDomainManifest(sduuid)
Line 67: 
Line 68:         # Just use a dict for the metadata
I think the code explain itself, lets remove the comments.
Line 69:         manifest.bind(dict())
Line 70:         return manifest
Line 71: 
Line 72: 


Line 69:         manifest.bind(dict())
Line 70:         return manifest
Line 71: 
Line 72: 
Line 73: def make_metafile(metafile):
Lets rename it to make_fake_metafile and remove the 
comments.
Line 74:     os.makedirs(os.path.dirname(metafile))
Line 75:     with open(metafile, "w") as f:


https://gerrit.ovirt.org/#/c/41993/12/tests/storagefakelib.py
File tests/storagefakelib.py:

Line 1: # Copyright 2012 Red Hat, Inc.
2015
Line 2: #
Line 3: # This program is free software; you can redistribute it and/or modify
Line 4: # it under the terms of the GNU General Public License as published by
Line 5: # the Free Software Foundation; either version 2 of the License, or


-- 
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: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list