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

Reply via email to