Bug 694206 - Swallowed exceptions in client upload API
Summary: Swallowed exceptions in client upload API
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Pulp
Classification: Retired
Component: z_other
Version: unspecified
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
: Sprint 22
Assignee: Jeff Ortel
QA Contact: Preethi Thomas
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-06 17:53 UTC by Jay Dobies
Modified: 2012-02-24 20:17 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-02-24 20:16:21 UTC


Attachments (Terms of Use)

Description Jay Dobies 2011-04-06 17:53:07 UTC
In Momento.read and .delete, if an exception occurs it isn't even logged. In read especially, I can't imagine we want to proceed with whatever is using it if it was unable to be read, but that's what the code will do.

While you're in there, it's spelled "memento" not "momento". And on line 45:

  path = path = os.path.join(root, str(checksum))

The second path assignment is just sloppy.

Comment 1 Jeff Ortel 2011-04-06 21:22:16 UTC
In this case it's normal for the memento to not exist so the code is doing exactly what it is intended to do.  However, not catching a specific exception could hide a syntax error so I updated to catch (and discard) a specific exception.  Also, added a comment so that it is clearer that this is intended.  Testing for os.path.exists() or other such tests still leaves the door open for race conditions and further complicates error handling here.  The only error condition this could potentially hide is the user intentionally changing permissions on ~/.pulp/upload or the memento itself.  The result would be that the memento is ignored and an interrupted upload would just start from scratch.

As for Momento vs. Memento.  My bad.  Perhaps I was thinking of the "momento" (spanish).

The 2nd "sloppy" path assignment - that "typo" has been corrected.

Comment 2 Jeff Ortel 2011-04-11 14:08:08 UTC
build: 0.162

Comment 3 Jay Dobies 2011-04-13 19:23:01 UTC
Fixed in build 0.163.

Comment 4 Preethi Thomas 2011-09-02 14:45:51 UTC
Steps to verify

<jdob> - upload a file. a large one will make life easier on you since you'll have more time to break it
<jdob> - delete the temporary upload chunk
<jdob> so I think you can just delete ~/.pulp/upload
<jdob> then make sure the client log shows an exception

Comment 5 Preethi Thomas 2011-10-04 16:25:20 UTC
verified

[root@preethi ~]# rpm -q pulp
pulp-0.0.237-2.fc15.noarch


no exception in the log as per comment#1

Comment 6 Preethi Thomas 2012-02-24 20:16:21 UTC
Pulp v1.0 is released
Closed Current Release.

Comment 7 Preethi Thomas 2012-02-24 20:17:41 UTC
Pulp v1.0 is released.


Note You need to log in before you can comment on or make changes to this bug.