Merge lp:~juju/txzookeeper/basic-node-api into lp:txzookeeper
- basic-node-api
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 22 |
Proposed branch: | lp:~juju/txzookeeper/basic-node-api |
Merge into: | lp:txzookeeper |
Diff against target: |
554 lines (+545/-0) 2 files modified
txzookeeper/node.py (+229/-0) txzookeeper/tests/test_node.py (+316/-0) |
To merge this branch: | bzr merge lp:~juju/txzookeeper/basic-node-api |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gustavo Niemeyer | Approve | ||
Review via email: mp+25465@code.launchpad.net |
Commit message
Description of the change
a minimal node abstraction api, with get/set data/acl and node iteration.
- 19. By Kapil Thangavelu
-
node - remove subscriptions methods, add additional data accessors which also return on change deferreds
- 20. By Kapil Thangavelu
-
re-enable the subscription tests as data accessors with watches, add an additional test for bad version error handling.
- 21. By Kapil Thangavelu
-
rename exists_with_watch to exists_and_watch for consistency.
Kapil Thangavelu (hazmat) wrote : | # |
On Wed, 19 May 2010 05:43:22 -0400, Gustavo Niemeyer
<email address hidden> wrote:
> Review: Needs Fixing
> Just for the record, as we discussed over the phone:
>
> [1]
>
> The notification abstractions hide a bit the intention behind the way
> that the ZooKeeper API is designed: watchers are set up on data
> retrieval because it only makes sense to be told about changes once you
> know the existing value, and once changes happen the client must ask
> again about the value and reset the notification if it's still
> interested in knowing about changes.
>
> As a consequence of this model, fast subsequent changes won't create a
> swarm effect, because all the clients are told about is that the value
> isn't the same they originally knew about anymore, but more than one
> change may have happened, and that's still alright.
>
> With that in mind, I suggest having something like this, in addition to
> get_data() and get_children():
>
> def get_data_
> (...)
> return data_deferred, watch_deferred
>
> def get_children_
> (...)
> return children_deferred, watch_deferred
>
> This uses twisted deferreds as well.
i've gone and ahead and switched the node implementation to utilize the
accesssors with explicit watches. i ended up adding an additional one for
exists_and_watch, as its nesc. to detect node creation.
>
> [2]
>
> This isn't about a specific action required.. just a feeling I had.
>
> I understand the intention behind the internal keeping of the node
> versioning, and I think it's a good model to start with. It's just not
> entirely clear to me how we'll want these things to behave in practice,
> but we'll certainly figure it out.
>
best usage is still tbd but there should be some value to the abstraction
layer else we're just using the client api. i think we'll end up
revisiting with some more experience.
cheers,
Kapil
Kapil Thangavelu (hazmat) wrote : | # |
On Wed, 19 May 2010 05:43:22 -0400, Gustavo Niemeyer
<email address hidden> wrote:
> Review: Needs Fixing
> Just for the record, as we discussed over the phone:
>
> [1]
>
> The notification abstractions hide a bit the intention behind the way
> that the ZooKeeper API is designed: watchers are set up on data
> retrieval because it only makes sense to be told about changes once you
> know the existing value, and once changes happen the client must ask
> again about the value and reset the notification if it's still
> interested in knowing about changes.
>
> As a consequence of this model, fast subsequent changes won't create a
> swarm effect, because all the clients are told about is that the value
> isn't the same they originally knew about anymore, but more than one
> change may have happened, and that's still alright.
>
> With that in mind, I suggest having something like this, in addition to
> get_data() and get_children():
>
> def get_data_
> (...)
> return data_deferred, watch_deferred
>
> def get_children_
> (...)
> return children_deferred, watch_deferred
>
> This uses twisted deferreds as well.
I've gone ahead and implemented the data accessors with watches.
>
> [2]
>
> This isn't about a specific action required.. just a feeling I had.
>
> I understand the intention behind the internal keeping of the node
> versioning, and I think it's a good model to start with. It's just not
> entirely clear to me how we'll want these things to behave in practice,
> but we'll certainly figure it out.
noted. we'll revisit based on experience.
Gustavo Niemeyer (niemeyer) wrote : | # |
[3]
Thanks for the watch implementation. It looks sensible.
As we discussed, one thing I wonder is if we should change the watch parameter into an Event object with "node" (the object, rather than the path), "kind", and "status". Might potentially make it more comfortable to consume the event.
[4]
+ def __cmp__(self, other):
+ return cmp(self.path, other.path)
This should be turned into __eq__ I believe, otherwise we'll get weird calls like node1 < node2 working in an arbitrary way, when they should error out.
[5]
+ if self._node_exists:
+ # if the node is deleted while we have this cached state we should
+ # covert the set to a create. XXX ??? Should we
On a second look, this starts to smell a bit strange. If we look at the algorithms created around ZooKeeper, they're generally very sensitive about when things are created, changed, deleted, etc. Having our Node API doing things behind the programmer's back feels like a hole we shouldn't get into. It reminds me of the sqlite Python wrapper, which does a lot of "smart" magic with the transaction, and as a side effect turns transactions into something unreliable (which is very ironic).
I understand and agree that having a create() method in a Node object which already exists may feel a bit weird from an OO perspective, but I really think we should go towards this way if we want to make the logic being created on top of this really easy to follow.
If we don't do this, we'll always to have to keep in mind "Ohh, this method call actually *creates* the node too!" when thinking of higher-level algorithms on top of ZooKeeper, which feels undesirable.
What do you think?
- 22. By Kapil Thangavelu
-
use failure trap for excpetion handlers, properties for node name&path.
- 23. By Kapil Thangavelu
-
explicit create method, explicit errors on get data, and set data on non existant nodes.
- 24. By Kapil Thangavelu
-
node no longer tracks existance, just last node version stat
- 25. By Kapil Thangavelu
-
node event object to aggregate watch callback values
- 26. By Kapil Thangavelu
-
node event encapsulates node object that the watch was fired on
Kapil Thangavelu (hazmat) wrote : | # |
[3] There's now an event object wrapping the watch callback values along with the node object.
[4] This is for sorting node collections, and is deterministic. It might be uintended in the node1 < node2 case. Alternative would be either providing a list compare function in the module, or letting users define one. Providing it on the class directly depends on how common the sorting nodes would be vs. confusion on other __cmp__ usages.
[5] All the implicit stuff has been yanked except the last version tracking. There is now an explicit create method on the node.
- 27. By Kapil Thangavelu
-
test for nodeevent repr
Gustavo Niemeyer (niemeyer) wrote : | # |
Nice improvements again, thank you!
Only one comment, and +1!
[6]
+ def __init__(self, event_type, connection_state, node):
+ self.type = event_type
+ self.connection
+ self.node = node
+ self.path = node.path
+
+ @property
+ def kind(self):
+ return self.kind_
It'd be nice to have the naming convention consistent here. event.type returns an integer, and event.kind returns a string, but they both mean exactly the same thing with a different data type. As a side effect, a few weeks from now it will be hard to remember whether the string is the type or the kind. I suggest something like kind and kind_name, or type and type_name.
Preview Diff
1 | === added file 'txzookeeper/node.py' |
2 | --- txzookeeper/node.py 1970-01-01 00:00:00 +0000 |
3 | +++ txzookeeper/node.py 2010-06-11 16:18:25 +0000 |
4 | @@ -0,0 +1,229 @@ |
5 | +from zookeeper import NoNodeException, BadVersionException |
6 | +from twisted.internet.defer import Deferred |
7 | +from txzookeeper.client import ZOO_OPEN_ACL_UNSAFE |
8 | + |
9 | + |
10 | +class NodeEvent(object): |
11 | + """ |
12 | + A node event is returned when a watch deferred fires. It denotes some event |
13 | + on the zookeeper node that the watch was requested on. |
14 | + |
15 | + @ivar path: Path to the node the event was about. |
16 | + |
17 | + @ivar type: An integer corresponding to the event type. The symbolic |
18 | + name mapping is available from the zookeeper module attributes. For |
19 | + convience one is included on the C{NodeEvent} class. |
20 | + |
21 | + @ivar kind: A symbolic name for the event's type. |
22 | + |
23 | + @ivar connection_state: integer representing the state of the |
24 | + zookeeper connection. |
25 | + """ |
26 | + |
27 | + __slots__ = ('type', 'connection_state', 'path', 'node') |
28 | + |
29 | + kind_map = { |
30 | + 1: 'created', |
31 | + 2: 'deleted', |
32 | + 3: 'changed', |
33 | + 4: 'child'} |
34 | + |
35 | + def __init__(self, event_type, connection_state, node): |
36 | + self.type = event_type |
37 | + self.connection_state = connection_state |
38 | + self.node = node |
39 | + self.path = node.path |
40 | + |
41 | + @property |
42 | + def kind(self): |
43 | + return self.kind_map[self.type] |
44 | + |
45 | + def __repr__(self): |
46 | + return "<NodeEvent %s at %r>"%(self.kind, self.path) |
47 | + |
48 | + |
49 | +class ZNode(object): |
50 | + """ |
51 | + A minimal object abstraction over a zookeeper node, utilizes no caching |
52 | + outside of trying to keep track of node existance and the last read |
53 | + version. It will attempt to utilize the last read version when modifying |
54 | + the node. On a bad version exception this values are cleared and the |
55 | + except reraised (errback chain continue.) |
56 | + """ |
57 | + |
58 | + def __init__(self, path, context): |
59 | + self._path = path |
60 | + self._name = path.split("/")[-1] |
61 | + self._context = context |
62 | + self._node_stat = None |
63 | + |
64 | + @property |
65 | + def path(self): |
66 | + """ |
67 | + The path to the node from the zookeeper root. |
68 | + """ |
69 | + return self._path |
70 | + |
71 | + @property |
72 | + def name(self): |
73 | + """ |
74 | + The name of the node in its container. |
75 | + """ |
76 | + return self._name |
77 | + |
78 | + def _get_version(self): |
79 | + if not self._node_stat: |
80 | + return -1 |
81 | + return self._node_stat["version"] |
82 | + |
83 | + def _on_error_bad_version(self, failure): |
84 | + failure.trap(BadVersionException) |
85 | + self._node_stat = None |
86 | + return failure |
87 | + |
88 | + def create(self, data="", acl=None, flags=0): |
89 | + """ |
90 | + Create the node with the given data, acl, and persistence flags. If no |
91 | + acl is given the default public acl is used. The default creation flag |
92 | + is persistent. |
93 | + """ |
94 | + if acl is None: |
95 | + acl = [ZOO_OPEN_ACL_UNSAFE] |
96 | + d = self._context.create(self.path, data, acl, flags) |
97 | + return d |
98 | + |
99 | + def _on_exists_success(self, node_stat): |
100 | + if node_stat is None: |
101 | + return False |
102 | + self._node_stat = node_stat |
103 | + return True |
104 | + |
105 | + def exists(self): |
106 | + """ |
107 | + Does the node exist or not, returns a boolean. |
108 | + """ |
109 | + d = self._context.exists(self.path) |
110 | + d.addCallback(self._on_exists_success) |
111 | + return d |
112 | + |
113 | + def exists_and_watch(self): |
114 | + """ |
115 | + Returns a boolean based on the node's existence. Also returns a |
116 | + deferred that fires when the node is modified/created/added/deleted. |
117 | + """ |
118 | + node_changed = Deferred() |
119 | + |
120 | + def on_node_event(event, state, path): |
121 | + return node_changed.callback( |
122 | + NodeEvent(event, state, self)) |
123 | + |
124 | + d = self._context.exists(self.path, on_node_event) |
125 | + d.addCallback(self._on_exists_success) |
126 | + return d, node_changed |
127 | + |
128 | + def _on_get_node_error(self, failure): |
129 | + failure.trap(NoNodeException) |
130 | + self._node_stat = None |
131 | + return failure |
132 | + |
133 | + def _on_get_node_success(self, data): |
134 | + (node_data, node_stat) = data |
135 | + self._node_stat = node_stat |
136 | + return node_data |
137 | + |
138 | + def get_data(self): |
139 | + """Retrieve the node's data.""" |
140 | + d = self._context.get(self.path) |
141 | + d.addCallback(self._on_get_node_success) |
142 | + d.addErrback(self._on_get_node_error) |
143 | + return d |
144 | + |
145 | + def get_data_and_watch(self): |
146 | + """ |
147 | + Retrieve the node's data and a deferred that fires when this data |
148 | + changes. |
149 | + """ |
150 | + node_changed = Deferred() |
151 | + |
152 | + def on_node_change(event, status, path): |
153 | + node_changed.callback( |
154 | + NodeEvent(event, status, self)) |
155 | + |
156 | + d = self._context.get(self.path, watcher=on_node_change) |
157 | + d.addCallback(self._on_get_node_success) |
158 | + d.addErrback(self._on_get_node_error) |
159 | + return d, node_changed |
160 | + |
161 | + def set_data(self, data): |
162 | + """Set the node's data.""" |
163 | + |
164 | + def on_success(value): |
165 | + self._node_stat = None |
166 | + return self |
167 | + |
168 | + version = self._get_version() |
169 | + d = self._context.set(self.path, data, version) |
170 | + d.addErrback(self._on_error_bad_version) |
171 | + d.addCallback(on_success) |
172 | + return d |
173 | + |
174 | + def get_acl(self): |
175 | + """ |
176 | + Get the ACL for this node. An ACL is a list of access control |
177 | + entity dictionaries. |
178 | + """ |
179 | + |
180 | + def on_success((acl, stat)): |
181 | + if stat is not None: |
182 | + self._node_stat = stat |
183 | + return acl |
184 | + d = self._context.get_acl(self.path) |
185 | + d.addCallback(on_success) |
186 | + return d |
187 | + |
188 | + def set_acl(self, acl): |
189 | + """ |
190 | + Set the ACL for this node. |
191 | + """ |
192 | + d = self._context.set_acl( |
193 | + self.path, acl, self._get_version()) |
194 | + d.addErrback(self._on_error_bad_version) |
195 | + return d |
196 | + |
197 | + def _on_get_children_filter_results(self, children, prefix): |
198 | + if prefix: |
199 | + children = [ |
200 | + name for name in children if name.startswith(prefix)] |
201 | + return [ |
202 | + self.__class__("/".join((self.path, name)), self._context) |
203 | + for name in children] |
204 | + |
205 | + def get_children(self, prefix=None): |
206 | + """ |
207 | + Get the children of this node, as ZNode objects. Optionally |
208 | + a name prefix may be passed which the child node must abide. |
209 | + """ |
210 | + d = self._context.get_children(self.path) |
211 | + d.addCallback(self._on_get_children_filter_results, prefix) |
212 | + return d |
213 | + |
214 | + def get_children_and_watch(self, prefix=None): |
215 | + """ |
216 | + Get the children of this node, as ZNode objects, and also return |
217 | + a deferred that fires if a child is added or deleted. Optionally |
218 | + a name prefix may be passed which the child node must abide. |
219 | + """ |
220 | + children_changed = Deferred() |
221 | + |
222 | + def on_child_added_removed(event, status, path): |
223 | + # path is the container not the child. |
224 | + children_changed.callback( |
225 | + NodeEvent(event, status, self)) |
226 | + |
227 | + d = self._context.get_children( |
228 | + self.path, watcher=on_child_added_removed) |
229 | + d.addCallback(self._on_get_children_filter_results, prefix) |
230 | + return d, children_changed |
231 | + |
232 | + def __cmp__(self, other): |
233 | + return cmp(self.path, other.path) |
234 | |
235 | === added file 'txzookeeper/tests/test_node.py' |
236 | --- txzookeeper/tests/test_node.py 1970-01-01 00:00:00 +0000 |
237 | +++ txzookeeper/tests/test_node.py 2010-06-11 16:18:25 +0000 |
238 | @@ -0,0 +1,316 @@ |
239 | +import zookeeper |
240 | +import hashlib |
241 | +import base64 |
242 | + |
243 | +from twisted.internet.defer import Deferred, inlineCallbacks |
244 | +from twisted.python.failure import Failure |
245 | +from txzookeeper.node import ZNode |
246 | +from txzookeeper.tests import TestCase |
247 | +from txzookeeper.tests.utils import deleteTree |
248 | +from txzookeeper import ZookeeperClient |
249 | + |
250 | + |
251 | +class NodeTest(TestCase): |
252 | + |
253 | + def setUp(self): |
254 | + super(NodeTest, self).setUp() |
255 | + zookeeper.set_debug_level(zookeeper.LOG_LEVEL_ERROR) |
256 | + self.client = ZookeeperClient("127.0.0.1:2181", 2000) |
257 | + d = self.client.connect() |
258 | + self.client2 = None |
259 | + |
260 | + def create_zoo(client): |
261 | + client.create("/zoo") |
262 | + d.addCallback(create_zoo) |
263 | + return d |
264 | + |
265 | + def tearDown(self): |
266 | + super(NodeTest, self).tearDown() |
267 | + deleteTree(handle=self.client.handle) |
268 | + if self.client.connected: |
269 | + self.client.close() |
270 | + if self.client2 and self.client2.connected: |
271 | + self.client2.close() |
272 | + zookeeper.set_debug_level(zookeeper.LOG_LEVEL_DEBUG) |
273 | + |
274 | + def _make_digest_identity(self, credentials): |
275 | + user, password = credentials.split(":") |
276 | + digest = hashlib.new("sha1", credentials).digest() |
277 | + return "%s:%s"%(user, base64.b64encode(digest)) |
278 | + |
279 | + def test_node_name_and_path(self): |
280 | + """ |
281 | + Each node has name and path. |
282 | + """ |
283 | + node = ZNode("/zoo/rabbit", self.client) |
284 | + self.assertEqual(node.name, "rabbit") |
285 | + self.assertEqual(node.path, "/zoo/rabbit") |
286 | + |
287 | + @inlineCallbacks |
288 | + def test_node_exists_nonexistant(self): |
289 | + """ |
290 | + A node knows whether it exists or not. |
291 | + """ |
292 | + node = ZNode("/zoo/rabbit", self.client) |
293 | + exists = yield node.exists() |
294 | + self.assertFalse(exists) |
295 | + |
296 | + @inlineCallbacks |
297 | + def test_node_set_data_on_nonexistant(self): |
298 | + """ |
299 | + Setting data on a non existant node raises a no node exception. |
300 | + """ |
301 | + node = ZNode("/zoo/rabbit", self.client) |
302 | + d = node.set_data("big furry ears") |
303 | + self.failUnlessFailure(d, zookeeper.NoNodeException) |
304 | + yield d |
305 | + |
306 | + @inlineCallbacks |
307 | + def test_node_create_set_data(self): |
308 | + """ |
309 | + A node can be created and have its data set. |
310 | + """ |
311 | + node = ZNode("/zoo/rabbit", self.client) |
312 | + data = "big furry ears" |
313 | + yield node.create(data) |
314 | + exists = yield self.client.exists("/zoo/rabbit") |
315 | + self.assertTrue(exists) |
316 | + node_data = yield node.get_data() |
317 | + self.assertEqual(data, node_data) |
318 | + data = data*2 |
319 | + |
320 | + yield node.set_data(data) |
321 | + node_data = yield node.get_data() |
322 | + self.assertEqual(data, node_data) |
323 | + |
324 | + @inlineCallbacks |
325 | + def test_node_get_data(self): |
326 | + """ |
327 | + Data can be fetched from a node. |
328 | + """ |
329 | + yield self.client.create("/zoo/giraffe", "mouse") |
330 | + data = yield ZNode("/zoo/giraffe", self.client).get_data() |
331 | + self.assertEqual(data, "mouse") |
332 | + |
333 | + @inlineCallbacks |
334 | + def test_node_get_data_nonexistant(self): |
335 | + """ |
336 | + Attempting to fetch data from a nonexistant node returns |
337 | + a non existant error. |
338 | + """ |
339 | + d = ZNode("/zoo/giraffe", self.client).get_data() |
340 | + self.failUnlessFailure(d, zookeeper.NoNodeException) |
341 | + yield d |
342 | + |
343 | + @inlineCallbacks |
344 | + def test_node_get_acl(self): |
345 | + """ |
346 | + The ACL for a node can be retrieved. |
347 | + """ |
348 | + yield self.client.create("/zoo/giraffe") |
349 | + acl = yield ZNode("/zoo/giraffe", self.client).get_acl() |
350 | + self.assertEqual(len(acl), 1) |
351 | + self.assertEqual(acl[0]['scheme'], 'world') |
352 | + |
353 | + def test_node_get_acl_nonexistant(self): |
354 | + """ |
355 | + The fetching the ACL for a non-existant node results in an error. |
356 | + """ |
357 | + node = ZNode("/zoo/giraffe", self.client) |
358 | + |
359 | + def assert_failed(failed): |
360 | + if not isinstance(failed, Failure): |
361 | + self.fail("Should have failed") |
362 | + self.assertTrue( |
363 | + isinstance(failed.value, zookeeper.NoNodeException)) |
364 | + d = node.get_acl() |
365 | + d.addBoth(assert_failed) |
366 | + return d |
367 | + |
368 | + @inlineCallbacks |
369 | + def test_node_set_acl(self): |
370 | + """ |
371 | + The ACL for a node can be modified. |
372 | + """ |
373 | + path = yield self.client.create("/zoo/giraffe") |
374 | + credentials = "zebra:moon" |
375 | + acl = [{"id": self._make_digest_identity(credentials), |
376 | + "scheme": "digest", |
377 | + "perms":zookeeper.PERM_ALL}] |
378 | + node = ZNode(path, self.client) |
379 | + # little hack around slow auth issue 770 zookeeper |
380 | + d = self.client.add_auth("digest", credentials) |
381 | + yield node.set_acl(acl) |
382 | + yield d |
383 | + node_acl, stat = yield self.client.get_acl(path) |
384 | + self.assertEqual(node_acl, acl) |
385 | + |
386 | + @inlineCallbacks |
387 | + def test_node_set_data_update_with_cached_exists(self): |
388 | + """ |
389 | + Data can be set on an existing node, updating it |
390 | + in place. |
391 | + """ |
392 | + node = ZNode("/zoo/monkey", self.client) |
393 | + yield self.client.create("/zoo/monkey", "stripes") |
394 | + exists = yield node.exists() |
395 | + self.assertTrue(exists) |
396 | + yield node.set_data("banana") |
397 | + data, stat = yield self.client.get("/zoo/monkey") |
398 | + self.assertEqual(data, "banana") |
399 | + |
400 | + @inlineCallbacks |
401 | + def test_node_set_data_update_with_invalid_cached_exists(self): |
402 | + """ |
403 | + If a node is deleted, attempting to set data on it |
404 | + raises a no node exception. |
405 | + """ |
406 | + node = ZNode("/zoo/monkey", self.client) |
407 | + yield self.client.create("/zoo/monkey", "stripes") |
408 | + exists = yield node.exists() |
409 | + self.assertTrue(exists) |
410 | + yield self.client.delete("/zoo/monkey") |
411 | + d = node.set_data("banana") |
412 | + self.failUnlessFailure(d, zookeeper.NoNodeException) |
413 | + yield d |
414 | + |
415 | + @inlineCallbacks |
416 | + def test_node_set_data_update_with_exists(self): |
417 | + """ |
418 | + Data can be set on an existing node, updating it |
419 | + in place. |
420 | + """ |
421 | + node = ZNode("/zoo/monkey", self.client) |
422 | + yield self.client.create("/zoo/monkey", "stripes") |
423 | + yield node.set_data("banana") |
424 | + data, stat = yield self.client.get("/zoo/monkey") |
425 | + self.assertEqual(data, "banana") |
426 | + |
427 | + @inlineCallbacks |
428 | + def test_node_exists_with_watch_nonexistant(self): |
429 | + """ |
430 | + The node's existance can be checked with the exist_watch api |
431 | + a deferred will be returned and any node level events, |
432 | + created, deleted, modified invoke the callback. You can |
433 | + get these create event callbacks for non existant nodes. |
434 | + """ |
435 | + node = ZNode("/zoo/elephant", self.client) |
436 | + exists, watch = yield node.exists_and_watch() |
437 | + |
438 | + self.client.create("/zoo/elephant") |
439 | + event = yield watch |
440 | + self.assertEqual(event.type, zookeeper.CREATED_EVENT) |
441 | + self.assertEqual(event.path, node.path) |
442 | + |
443 | + @inlineCallbacks |
444 | + def test_node_get_data_with_watch_on_update(self): |
445 | + """ |
446 | + Subscribing to a node will get node update events. |
447 | + """ |
448 | + yield self.client.create("/zoo/elephant") |
449 | + |
450 | + node = ZNode("/zoo/elephant", self.client) |
451 | + data, watch = yield node.get_data_and_watch() |
452 | + yield self.client.set("/zoo/elephant") |
453 | + event = yield watch |
454 | + self.assertEqual(event.type, zookeeper.CHANGED_EVENT) |
455 | + self.assertEqual(event.path, "/zoo/elephant") |
456 | + |
457 | + @inlineCallbacks |
458 | + def test_node_get_data_with_watch_on_delete(self): |
459 | + """ |
460 | + Subscribing to a node will get node deletion events. |
461 | + """ |
462 | + yield self.client.create("/zoo/elephant") |
463 | + |
464 | + node = ZNode("/zoo/elephant", self.client) |
465 | + data, watch = yield node.get_data_and_watch() |
466 | + |
467 | + yield self.client.delete("/zoo/elephant") |
468 | + event = yield watch |
469 | + self.assertEqual(event.type, zookeeper.DELETED_EVENT) |
470 | + self.assertEqual(event.path, "/zoo/elephant") |
471 | + |
472 | + @inlineCallbacks |
473 | + def test_node_children(self): |
474 | + """ |
475 | + A node's children can be introspected. |
476 | + """ |
477 | + node = ZNode("/zoo", self.client) |
478 | + node_path_a = yield self.client.create("/zoo/lion") |
479 | + node_path_b = yield self.client.create("/zoo/tiger") |
480 | + children = yield node.get_children() |
481 | + children.sort() |
482 | + self.assertEqual(children[0].path, node_path_a) |
483 | + self.assertEqual(children[1].path, node_path_b) |
484 | + |
485 | + @inlineCallbacks |
486 | + def test_node_children_by_prefix(self): |
487 | + """ |
488 | + A node's children can be introspected optionally with a prefix. |
489 | + """ |
490 | + node = ZNode("/zoo", self.client) |
491 | + node_path_a = yield self.client.create("/zoo/lion") |
492 | + yield self.client.create("/zoo/tiger") |
493 | + children = yield node.get_children("lion") |
494 | + children.sort() |
495 | + self.assertEqual(children[0].path, node_path_a) |
496 | + self.assertEqual(len(children), 1) |
497 | + |
498 | + @inlineCallbacks |
499 | + def test_node_get_children_with_watch_create(self): |
500 | + """ |
501 | + A node's children can explicitly be watched to given existance |
502 | + events for node creation and destruction. |
503 | + """ |
504 | + node = ZNode("/zoo", self.client) |
505 | + children, watch = yield node.get_children_and_watch() |
506 | + yield self.client.create("/zoo/lion") |
507 | + event = yield watch |
508 | + self.assertEqual(event.path, "/zoo") |
509 | + self.assertEqual(event.type, zookeeper.CHILD_EVENT) |
510 | + self.assertEqual(event.kind, "child") |
511 | + |
512 | + @inlineCallbacks |
513 | + def test_node_get_children_with_watch_delete(self): |
514 | + """ |
515 | + A node's children can explicitly be watched to given existance |
516 | + events for node creation and destruction. |
517 | + """ |
518 | + node = ZNode("/zoo", self.client) |
519 | + yield self.client.create("/zoo/lion") |
520 | + children, watch = yield node.get_children_and_watch() |
521 | + yield self.client.delete("/zoo/lion") |
522 | + event = yield watch |
523 | + self.assertEqual(event.path, "/zoo") |
524 | + self.assertEqual(event.type, zookeeper.CHILD_EVENT) |
525 | + self.assertEqual(repr(event), |
526 | + "<NodeEvent child at '/zoo'>") |
527 | + |
528 | + @inlineCallbacks |
529 | + def test_bad_version_error(self): |
530 | + """ |
531 | + The node captures the node version on any read operations, which |
532 | + it utilizes for write operations. On a concurrent modification error |
533 | + the node return a bad version error, this also clears the cached |
534 | + state so subsequent modifications will be against the latest version, |
535 | + unless the cache is seeded again by a read operation. |
536 | + """ |
537 | + node = ZNode("/zoo/lion", self.client) |
538 | + |
539 | + self.client2 = ZookeeperClient("127.0.0.1:2181") |
540 | + yield self.client2.connect() |
541 | + |
542 | + yield self.client.create("/zoo/lion", "mouse") |
543 | + yield node.get_data() |
544 | + yield self.client2.set("/zoo/lion", "den2") |
545 | + data = yield self.client.exists("/zoo/lion") |
546 | + self.assertEqual(data['version'], 1) |
547 | + d = node.set_data("zebra") |
548 | + self.failUnlessFailure(d, zookeeper.BadVersionException) |
549 | + yield d |
550 | + |
551 | + # after failure the cache is deleted, and a set proceeds |
552 | + yield node.set_data("zebra") |
553 | + data = yield node.get_data() |
554 | + self.assertEqual(data, "zebra") |
Just for the record, as we discussed over the phone:
[1]
The notification abstractions hide a bit the intention behind the way that the ZooKeeper API is designed: watchers are set up on data retrieval because it only makes sense to be told about changes once you know the existing value, and once changes happen the client must ask again about the value and reset the notification if it's still interested in knowing about changes.
As a consequence of this model, fast subsequent changes won't create a swarm effect, because all the clients are told about is that the value isn't the same they originally knew about anymore, but more than one change may have happened, and that's still alright.
With that in mind, I suggest having something like this, in addition to get_data() and get_children():
def get_data_ and_watch( self):
(...)
return data_deferred, watch_deferred
def get_children_ and_watch( self):
(...)
return children_deferred, watch_deferred
This uses twisted deferreds as well.
[2]
This isn't about a specific action required.. just a feeling I had.
I understand the intention behind the internal keeping of the node versioning, and I think it's a good model to start with. It's just not entirely clear to me how we'll want these things to behave in practice, but we'll certainly figure it out.