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