Nir Soffer has posted comments on this change. Change subject: gluster: Set mount path based on gluster volume info ......................................................................
Patch Set 4: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/48308/4/tests/storageServerTests.py File tests/storageServerTests.py: Line 150: Line 151: """ Line 152: This test simulates a use case where gluster server provided in the path Line 153: doesn't appear in the volume info. Line 154: """ Docstring should come under the function, bot before it. def func_name(): """ docstring here... """ Line 155: @MonkeyPatch(storageServer, 'supervdsm', FakeSupervdsm()) Line 156: def test_server_not_in_volinfo(self): Line 157: def glusterVolumeInfo(volname=None, volfileServer=None): Line 158: return {'music': {'brickCount': '3', https://gerrit.ovirt.org/#/c/48308/4/vdsm/storage/storageServer.py File vdsm/storage/storageServer.py: Line 339: servers = [brick.split(":")[0] for brick in self.volinfo['bricks']] Line 340: if self._volfileserver in servers: Line 341: servers.remove(self._volfileserver) Line 342: else: Line 343: self.log.warn("gluster server %s not in bricks, possibly mounting \ This does not work, should be: self.log.warning("first line..." "...continuation line") Also, including the servers in the log would make it easier to debug later. We can go and find the mount command later in the log, but why make it harder for us? Finally use %r for the server name, will reveal issues with leading and trailing whitespace. (e.g. server is " foo", " foo" in ["foo", "bar"] -> False) Line 344: duplicate servers", self._volfileserver) Line 345: Line 346: if not servers: Line 347: return "" -- To view, visit https://gerrit.ovirt.org/48308 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id3386b37cd407c52e1b8f38d54c236bffc143e2f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@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
