Adam Litke has posted comments on this change. Change subject: StorageDomainManifest: introduce class heirarchy ......................................................................
Patch Set 9: (1 comment) https://gerrit.ovirt.org/#/c/41993/9/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 402: def __init__(self, sdUUID): Line 403: domaindir = os.path.join(self.mountpoint, sdUUID) Line 404: sd.StorageDomainManifest.__init__(self, sdUUID, domaindir) Line 405: Line 406: def bind(self, metadata=None): > What will happen if you forget to call bind? Will every/some operation fail No, it will probably be an AttributeError. I'd rather not pollute the code with runtime checks to guard against a programming error. There are plenty of classes in vdsm that can be misused more easily than this one :) Sure, but lazy initialization causes other problems. It will become hard for class users to know which operations can be performed on the class without triggering the implicit bind. I'd prefer an explicit call to bind. Line 407: if metadata is None: Line 408: metadata = selectMetadata(self.sdUUID) Line 409: self.replaceMetadata(metadata) Line 410: -- 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
