Change in vdsm[master]: Handling removing file Exceptions.

List overview All Threads
Download

newer

older

Change in vdsm[master]: vdsm: pep8...

Change in vdsm[master]: Add...

ewarszaw＠redhat.com

17 Oct 2012 17 Oct '12

11 a.m.

Eduardo has uploaded a new change for review.

Change subject: Handling removing file Exceptions. ......................................................................

Handling removing file Exceptions.

Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Signed-off-by: Eduardo ewarszaw@redhat.com --- M vdsm/utils.py 1 file changed, 7 insertions(+), 2 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/25/8625/1

diff --git a/vdsm/utils.py b/vdsm/utils.py index 0dbb342..c6e8514 100644 --- a/vdsm/utils.py +++ b/vdsm/utils.py @@ -61,8 +61,13 @@ """ try: os.unlink(fileToRemove) - except: - pass + except OSError as e: + if e.errno == errno.ENOENT: + logging.warning("File: %s already removed", fileToRemove) + else: + logging.error("Removing file: %s failed", fileToRemove, + exc_info=True) + raise

def readMemInfo(): """

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange Gerrit-Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com

Show replies by date

Dan Kenigsberg

17 Oct 17 Oct

1:34 p.m.

Dan Kenigsberg has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 1: I would prefer that you didn't submit this

(2 inline comments)

You are perfectly right - but please tie the loose ends.

.................................................... File vdsm/utils.py Line 56: """ Line 57: Try to remove a file. Line 58: Line 59: .. note:: Line 60: If the operation fails the function exists silently. please update the docstring to fit the new-and-improved semantics.

""" Remove a file .. note:: If the file is not find, exit silently. """ Line 61: """ Line 62: try: Line 63: os.unlink(fileToRemove) Line 64: except OSError as e:

Line 64: except OSError as e: Line 65: if e.errno == errno.ENOENT: Line 66: logging.warning("File: %s already removed", fileToRemove) Line 67: else: Line 68: logging.error("Removing file: %s failed", fileToRemove, no need to log here, as you re-raise the exception. Line 69: exc_info=True) Line 70: raise Line 71: Line 72: def readMemInfo():

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com

agl＠us.ibm.com

5:03 p.m.

Adam Litke has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 3: I would prefer that you didn't submit this

(1 inline comment)

.................................................... File vdsm/utils.py Line 65: logging.warning("File: %s already removed", fileToRemove) Line 66: else: Line 67: logging.error("Removing file: %s failed", fileToRemove, Line 68: exc_info=True) Line 69: raise As Dan mentioned in an earlier patch series, you don't need to log here because you are raising the exception. Line 70: Line 71: def readMemInfo(): Line 72: """ Line 73: Parse ``/proc/meminfo`` and return its content as a dictionary.

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com

ewarszaw＠redhat.com

5:19 p.m.

Eduardo has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 3: (1 inline comment)

.................................................... File vdsm/utils.py Line 65: logging.warning("File: %s already removed", fileToRemove) Line 66: else: Line 67: logging.error("Removing file: %s failed", fileToRemove, Line 68: exc_info=True) Line 69: raise Since quite a lot of vdsm code silently swallow exceptions, we prefer to log here. Since it is the negative flow, should be exceptional and not blow the log. Line 70: Line 71: def readMemInfo(): Line 72: """ Line 73: Parse ``/proc/meminfo`` and return its content as a dictionary.

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

agl＠us.ibm.com

18 Oct 18 Oct

9:18 p.m.

Adam Litke has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 4: Looks good to me, but someone else must approve

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com

shuming＠linux.vnet.ibm.com

19 Oct 19 Oct

2:51 a.m.

Shu Ming has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 4: Looks good to me, but someone else must approve

(1 inline comment)

.................................................... Commit Message Line 3: AuthorDate: 2012-10-17 12:52:02 +0200 Line 4: Commit: Saggi Mizrahi smizrahi@redhat.com Line 5: CommitDate: 2012-10-18 16:25:37 -0400 Line 6: Line 7: Handling removing file Exceptions. Nit, It would be better to change the subject to "Handling removing file ENOENT exception" Line 8: Line 9: Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

ybronhei＠redhat.com

21 Oct 21 Oct

10:46 p.m.

Yaniv Bronhaim has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 4: Looks good to me, but someone else must approve

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

ykaplan＠redhat.com

18 Dec 18 Dec

6:10 p.m.

Yeela Kaplan has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 4: Looks good to me, but someone else must approve

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

abaron＠redhat.com

19 Dec 19 Dec

9:35 a.m.

Ayal Baron has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 4: Looks good to me, approved

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com

ewarszaw＠redhat.com

9:44 a.m.

Eduardo has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 4: Verified

Verified by Y. Bronheim, once upon a time.

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com

Dan Kenigsberg

9:49 a.m.

Dan Kenigsberg has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 4: I would prefer that you didn't submit this

(1 inline comment)

.................................................... File vdsm/utils.py Line 63: except OSError as e: Line 64: if e.errno == errno.ENOENT: Line 65: logging.warning("File: %s already removed", fileToRemove) Line 66: else: Line 67: logging.error("Removing file: %s failed", fileToRemove, this log is pointless as it would be logged again when the re-raised exception is caught.

if for some reason you want to keep, please fix the pep8 indentation here, or this would break build on f18. Line 68: exc_info=True) Line 69: raise Line 70: Line 71: def readMemInfo():

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com

Jenkins CI RO

10:32 a.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/474/ (1/2)

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment Gerrit-Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

10:32 a.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/439/ (2/2)

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Jenkins CI RO

10:34 a.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 5:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/439/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/474/ : SUCCESS

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Dan Kenigsberg

10:36 a.m.

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Handling removing file Exceptions. ......................................................................

Handling removing file Exceptions.

Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Signed-off-by: Eduardo ewarszaw@redhat.com --- M vdsm/utils.py 1 file changed, 8 insertions(+), 4 deletions(-)

Approvals: Dan Kenigsberg: Verified; Looks good to me, approved

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged Gerrit-Change-Id: If76e158ad69921d382e3d42ae45486534b9379f9 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server

Dan Kenigsberg

10:36 a.m.

Dan Kenigsberg has posted comments on this change.

Change subject: Handling removing file Exceptions. ......................................................................

Patch Set 5: Verified; Looks good to me, approved

reluctantly, copying score.

-- To view, visit http://gerrit.ovirt.org/8625 To unsubscribe, visit http://gerrit.ovirt.org/settings

4213

Age (days ago)

4276

Last active (days ago)

vdsm-patches@lists.fedorahosted.org

15 comments

8 participants

tags (0)

participants (8)

abaron＠redhat.com
agl＠us.ibm.com
Dan Kenigsberg
ewarszaw＠redhat.com
Jenkins CI RO
shuming＠linux.vnet.ibm.com
ybronhei＠redhat.com
ykaplan＠redhat.com