Merge lp://staging/~roadmr/checkbox/1097866-thread-arguments into lp://staging/checkbox

Proposed by Daniel Manrique
Status: Merged
Approved by: Jeff Marcom
Approved revision: 1876
Merged at revision: 1875
Proposed branch: lp://staging/~roadmr/checkbox/1097866-thread-arguments
Merge into: lp://staging/checkbox
Diff against target: 38 lines (+8/-3)
2 files modified
checkbox/contrib/REThread.py (+6/-3)
debian/changelog (+2/-0)
To merge this branch: bzr merge lp://staging/~roadmr/checkbox/1097866-thread-arguments
Reviewer Review Type Date Requested Status
Jeff Marcom (community) Approve
Daniel Manrique (community) Needs Resubmitting
Review via email: mp+142567@code.staging.launchpad.net

Description of the change

High-priority fix for the linked bug. Checkbox doesn't work in Raring without this :(

To post a comment you must log in.
Revision history for this message
Jeff Marcom (jeffmarcom) wrote :

I'd rather see the following personally:
class REThread(threading.Thread):
    '''Enhanced threading.Thread which can deliver a return value and propagate
    exceptions from the called thread to the calling thread.'''

    def __init__(self, group=None, target=None, name=None, args=(), kwargs={},
            verbose=None):
        '''Initialize Thread, identical to threading.Thread.__init__().'''
        #Note that due to api differences in python (verbose disappeared
        #at some point before 3.3 and got replaced by daemon), we just
        #ignore the verbose attribute. It's not used anywhere in checkbox
        #so this is safe for our purposes.
        self.__target = target
        self.__args = args
        self.__kwargs = kwargs
        self._retval = None
        self._exception = None

        threading.Thread.__init__(self)

Let me know if you think this won't work

review: Needs Information
1876. By Daniel Manrique

Updated as suggested by Jeff Marcom

Revision history for this message
Daniel Manrique (roadmr) wrote :

That's nicer, I tested it on the raring vm and it seems to work, so win!

review: Needs Resubmitting
Revision history for this message
Jeff Marcom (jeffmarcom) wrote :

Thanks, approved!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches