Change in vdsm[master]: ImageManifest: introduce ImageManifest class
nsoffer at redhat.com
nsoffer at redhat.com
Wed Sep 23 19:11:09 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: ImageManifest: introduce ImageManifest class
......................................................................
Patch Set 12: Code-Review-1
(3 comments)
https://gerrit.ovirt.org/#/c/43551/12/tests/sdm_indirection_tests.py
File tests/sdm_indirection_tests.py:
Line 283:
Line 284:
Line 285: class FakeImageManifest(image.ImageManifest):
Line 286: def __init__(self):
Line 287: self.repoPath = '/rhev/data-center'
We don't need this now - why not use the original signature and pass the repo path when you create the manifest?
Line 288:
Line 289: @recorded
Line 290: def getImageDir(self, sdUUID, imgUUID):
Line 291: pass
https://gerrit.ovirt.org/#/c/43551/12/vdsm/storage/image.py
File vdsm/storage/image.py:
Line 88:
Line 89:
Line 90: class ImageManifest(object):
Line 91: def __init__(self, repoPath):
Line 92: self.repoPath = repoPath
Why not readonly property?
Line 93:
Line 94: def getImageDir(self, sdUUID, imgUUID):
Line 95: """
Line 96: Return image directory
Line 118: cls.log.error("createImageRollback: Cannot remove dirty image "
Line 119: "folder %s" % (imageDir))
Line 120:
Line 121: def __init__(self, repoPath):
Line 122: self.manifest = ImageManifest(repoPath)
Why public? Are caller expected to be able to replace a manifest? Lets go with the default that everything is private. We will expose stuff using properties only when required.
Line 123:
Line 124: @property
Line 125: def repoPath(self):
Line 126: return self.manifest.repoPath
--
To view, visit https://gerrit.ovirt.org/43551
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7a78aef10902b9baf6f88d2e053e2232e2a1267
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: Jenkins CI
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