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
=== modified file 'bzrlib/cleanup.py'
--- bzrlib/cleanup.py 2010-03-03 22:59:21 +0000
+++ bzrlib/cleanup.py 2010-04-27 07:33:23 +0000
@@ -78,7 +78,28 @@
78 _run_cleanup(func, *args, **kwargs)78 _run_cleanup(func, *args, **kwargs)
7979
8080
81class OperationWithCleanups(object):81class ObjectWithCleanups(object):
82 """A mixin for objects that hold a cleanup list.
83
84 Subclass or client code can call add_cleanup and then later `cleanup_now`.
85 """
86 def __init__(self):
87 self.cleanups = deque()
88
89 def add_cleanup(self, cleanup_func, *args, **kwargs):
90 """Add a cleanup to run.
91
92 Cleanups may be added at any time.
93 Cleanups will be executed in LIFO order.
94 """
95 self.cleanups.appendleft((cleanup_func, args, kwargs))
96
97 def cleanup_now(self):
98 _run_cleanups(self.cleanups)
99 self.cleanups.clear()
100
101
102class OperationWithCleanups(ObjectWithCleanups):
82 """A way to run some code with a dynamic cleanup list.103 """A way to run some code with a dynamic cleanup list.
83104
84 This provides a way to add cleanups while the function-with-cleanups is105 This provides a way to add cleanups while the function-with-cleanups is
@@ -102,16 +123,8 @@
102 """123 """
103124
104 def __init__(self, func):125 def __init__(self, func):
126 super(OperationWithCleanups, self).__init__()
105 self.func = func127 self.func = func
106 self.cleanups = deque()
107
108 def add_cleanup(self, cleanup_func, *args, **kwargs):
109 """Add a cleanup to run.
110
111 Cleanups may be added at any time before or during the execution of
112 self.func. Cleanups will be executed in LIFO order.
113 """
114 self.cleanups.appendleft((cleanup_func, args, kwargs))
115128
116 def run(self, *args, **kwargs):129 def run(self, *args, **kwargs):
117 return _do_with_cleanups(130 return _do_with_cleanups(
@@ -121,10 +134,6 @@
121 return _do_with_cleanups(134 return _do_with_cleanups(
122 self.cleanups, self.func, *args, **kwargs)135 self.cleanups, self.func, *args, **kwargs)
123136
124 def cleanup_now(self):
125 _run_cleanups(self.cleanups)
126 self.cleanups.clear()
127
128137
129def _do_with_cleanups(cleanup_funcs, func, *args, **kwargs):138def _do_with_cleanups(cleanup_funcs, func, *args, **kwargs):
130 """Run `func`, then call all the cleanup_funcs.139 """Run `func`, then call all the cleanup_funcs.
131140
=== modified file 'bzrlib/tests/test_cleanup.py'
--- bzrlib/tests/test_cleanup.py 2010-03-03 22:59:21 +0000
+++ bzrlib/tests/test_cleanup.py 2010-04-27 07:33:23 +0000
@@ -20,6 +20,7 @@
20from bzrlib.cleanup import (20from bzrlib.cleanup import (
21 _do_with_cleanups,21 _do_with_cleanups,
22 _run_cleanup,22 _run_cleanup,
23 ObjectWithCleanups,
23 OperationWithCleanups,24 OperationWithCleanups,
24 )25 )
25from bzrlib.tests import TestCase26from bzrlib.tests import TestCase
@@ -276,3 +277,17 @@
276 [('func called', 'foo'), 'cleanup 1', 'cleanup 2', 'cleanup 3',277 [('func called', 'foo'), 'cleanup 1', 'cleanup 2', 'cleanup 3',
277 'cleanup 4'], call_log)278 'cleanup 4'], call_log)
278279
280
281class SampleWithCleanups(ObjectWithCleanups):
282
283 pass
284
285
286class TestObjectWithCleanups(TestCase):
287
288 def test_object_with_cleanups(self):
289 a = []
290 s = SampleWithCleanups()
291 s.add_cleanup(a.append, 42)
292 s.cleanup_now()
293 self.assertEqual(a, [42])