Code review comment for lp:~juju/txzookeeper/basic-node-api

Revision history for this message
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_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.

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.

« Back to merge proposal