Nir Soffer has posted comments on this change. Change subject: gluster: Refactor GlusterFSConnection class ......................................................................
Patch Set 3: (4 comments) https://gerrit.ovirt.org/#/c/40575/3/vdsm/storage/storageServer.py File vdsm/storage/storageServer.py: Line 166 Line 167 Line 168 Line 169 Line 170 Previously, we use to create the local path by: os.path.join(self.getLocalpathBase(), remote_path) And we customized the behavior by overriding self.getLocalpathBase(), accessing MountConnection.getLocalPathBase() We also have code to customize MountConnection.localPathBase during runtime, I don't see any need to support this. We used the term "localPath" instead of the more common term "mountpoint". This is clumsy and more complicated than needed. We can use this instead: class MountConnection(object): ROOT = "/tmp" DIR = "" Subclass can customized by overriding DIR. Line 199 Line 200 Line 201 Line 202 Line 203 Use: path = self._remotePath.replace("_", "__").replace("/", "_") return os.path.join(self.ROOT, self.DIR, path) Line 200: Line 201: def _getLocalPath(self): Line 202: localPath = self.getLocalPathBase() Line 203: if self._localPath: Line 204: localPath = self._localPath Not needed Line 205: Line 206: return os.path.join(localPath, Line 207: self._remotePath.replace("_", Line 208: "__").replace("/", "_")) Line 258: def __hash__(self): Line 259: return hash(type(self)) ^ hash(self._mount) Line 260: Line 261: Line 262: class GlusterFSConnection(object): Generally I prefer composition over sub-classing, but in this case we don't have any behavior in the subclass, so creating an object using MountConnection is wasted effort. The only differences between MountConnection and GlusterfsConnection are - "glusterSD" used in the mountpoint - "glusterfs" used for vfsType So a better change would be: GlusterFSConnection(MountConnection): VFSTYPE = "glusterfs" DIR = "gluserSD" And change MountConnection to use this class constants - see the comments in MountConnection. The other change planed is to modify options before connecting. We can override the options property in the subclass, and return a modified self._options. Line 263: Line 264: def __init__(self, spec, vfsType="glusterfs", options=""): Line 265: self._vfsType = vfsType Line 266: self._remotePath = spec -- To view, visit https://gerrit.ovirt.org/40575 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a55a7f219a60c7ffb0ce981175492e560bb757f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches