Bazaar

Merge lp:~mbp/bzr/cleanup into lp:bzr

  1. cleanup
  2. Merge into bzr.dev
Proposed by Martin Pool
Status: Merged
Approved by: Martin Pool
Approved revision: no longer in the source branch.
Merged at revision: 5295
Proposed branch: lp:~mbp/bzr/cleanup
Merge into: lp:bzr
Diff against target: 92 lines (+38/-14)
2 files modified
bzrlib/cleanup.py (+23/-14)
bzrlib/tests/test_cleanup.py (+15/-0)
To merge this branch: bzr merge lp:~mbp/bzr/cleanup
Reviewer Review Type Date Requested Status
Andrew Bennetts Approve
Review via email: mp+24200@code.launchpad.net

Commit message

add ObjectWithCleanups

Description of the change

This lets the cleanup code be used when the cleanups should be implicitly be done through some other operation on the object.

To post a comment you must log in.
Revision history for this message
Andrew Bennetts (spiv) wrote :

Hmm. I think this is fine, although it feels half-done to me, because many objects that want cleanup management (e.g. Command, Commit) also want a robust way to run a method and then the cleanups.

So I'm ok with this landing if you find it useful for something, but I anticipate that it will soon grow some equivalent to the run method of OperationWithCleanups too (or perhaps _do_with_cleanups will be promoted to a public function?). Also, the cleanup.py module docstring is pretty focused on the robust-run-with-cleanups use case, so perhaps if the scope is growing to more general cleanup-list management then the docstring should be updated to reflect that.

review: Approve
Revision history for this message
Vincent Ladeuil (vila) wrote :

@Martin: So, Approved or Work In Progress ?

Revision history for this message
Martin Pool (mbp) wrote :

It should probably wait until I'm sure it does something I specifically want. Thanks for keeping the queue down.

Revision history for this message
Martin Pool (mbp) wrote :

I did find this useful later, so on the weight of spiv's earlier +Approved I'm going to send it up to pqm.

I agree that often one would want "do this and then clean up", but I think you can also have cases where cleanup_now will be called from eg a tearDown method, and it's somebody else's job to arrange for that to be called.

Revision history for this message
Martin Pool (mbp) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/cleanup.py'
2--- bzrlib/cleanup.py 2010-03-03 22:59:21 +0000
3+++ bzrlib/cleanup.py 2010-04-27 07:33:23 +0000
4@@ -78,7 +78,28 @@
5 _run_cleanup(func, *args, **kwargs)
6
7
8-class OperationWithCleanups(object):
9+class ObjectWithCleanups(object):
10+ """A mixin for objects that hold a cleanup list.
11+
12+ Subclass or client code can call add_cleanup and then later `cleanup_now`.
13+ """
14+ def __init__(self):
15+ self.cleanups = deque()
16+
17+ def add_cleanup(self, cleanup_func, *args, **kwargs):
18+ """Add a cleanup to run.
19+
20+ Cleanups may be added at any time.
21+ Cleanups will be executed in LIFO order.
22+ """
23+ self.cleanups.appendleft((cleanup_func, args, kwargs))
24+
25+ def cleanup_now(self):
26+ _run_cleanups(self.cleanups)
27+ self.cleanups.clear()
28+
29+
30+class OperationWithCleanups(ObjectWithCleanups):
31 """A way to run some code with a dynamic cleanup list.
32
33 This provides a way to add cleanups while the function-with-cleanups is
34@@ -102,16 +123,8 @@
35 """
36
37 def __init__(self, func):
38+ super(OperationWithCleanups, self).__init__()
39 self.func = func
40- self.cleanups = deque()
41-
42- def add_cleanup(self, cleanup_func, *args, **kwargs):
43- """Add a cleanup to run.
44-
45- Cleanups may be added at any time before or during the execution of
46- self.func. Cleanups will be executed in LIFO order.
47- """
48- self.cleanups.appendleft((cleanup_func, args, kwargs))
49
50 def run(self, *args, **kwargs):
51 return _do_with_cleanups(
52@@ -121,10 +134,6 @@
53 return _do_with_cleanups(
54 self.cleanups, self.func, *args, **kwargs)
55
56- def cleanup_now(self):
57- _run_cleanups(self.cleanups)
58- self.cleanups.clear()
59-
60
61 def _do_with_cleanups(cleanup_funcs, func, *args, **kwargs):
62 """Run `func`, then call all the cleanup_funcs.
63
64=== modified file 'bzrlib/tests/test_cleanup.py'
65--- bzrlib/tests/test_cleanup.py 2010-03-03 22:59:21 +0000
66+++ bzrlib/tests/test_cleanup.py 2010-04-27 07:33:23 +0000
67@@ -20,6 +20,7 @@
68 from bzrlib.cleanup import (
69 _do_with_cleanups,
70 _run_cleanup,
71+ ObjectWithCleanups,
72 OperationWithCleanups,
73 )
74 from bzrlib.tests import TestCase
75@@ -276,3 +277,17 @@
76 [('func called', 'foo'), 'cleanup 1', 'cleanup 2', 'cleanup 3',
77 'cleanup 4'], call_log)
78
79+
80+class SampleWithCleanups(ObjectWithCleanups):
81+
82+ pass
83+
84+
85+class TestObjectWithCleanups(TestCase):
86+
87+ def test_object_with_cleanups(self):
88+ a = []
89+ s = SampleWithCleanups()
90+ s.add_cleanup(a.append, 42)
91+ s.cleanup_now()
92+ self.assertEqual(a, [42])