Saggi Mizrahi has posted comments on this change. Change subject: ioprocess: Move and reword rename() warning log line ......................................................................
Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/32194/1/vdsm/storage/outOfProcess.py File vdsm/storage/outOfProcess.py: Line 229: if log.isEnabledFor(logging.DEBUG): Line 230: log.warn("*DEPRECATED* Renaming a non-empty directory is not " Line 231: "an atomic operation. It is only supported for backward " Line 232: "compatibility with existing algorithms and will be " Line 233: "removed when possible.") > logs are not the place for warnings - we have warnings module for this: This would make the error appear in stderr where no one looks. It would also make it appear in syslog for customers. It also forces me to change things in every method that uses this incorrectly to add warning suppression clauses. All of those places are in storage code that is very hard to test. But I'm going to change it. I hope this makes you feel good you wasted everyone's time and produced something that is functionally inferior so you would have something to complain about in the most insignificant corner of the codebase. When you ask someone to take out a log I hope you try and take the log out of your own eye first. Line 234: Line 235: _IOProcessFileUtils(self._iop).cleanupdir(newpath, False) Line 236: self.mkdir(newpath) Line 237: for fname in self.listdir(oldpath): -- To view, visit http://gerrit.ovirt.org/32194 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d83f91803515845c993c01c23390aaf1d757dff Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches