Code review comment for lp:~hazmat/txzookeeper/distributed-queue

Thanks Kapil. Queue looks great. Some items about the new stuff:

[15]

+ and processed in the order they where placed in the queue. (TODO) An
+ implementation with less contention between consumers might instead utilize
+ a reliable queue with a lock.

Even without the lock, a better implementation would wait for the removal of the specific -processing file which is holding further action back, rather than any change in the queue. E.g. if the item itself is removed, but not the -processing control file, it will fire off, and fail again. Also (and most importantly), any new items added to the queue will also fire the watch and cause the consumer to refetch the full child list only to find out that it's still being processed by someone else, and it can do nothing about the new item appended at the back of the queue.

[16]

+class SerializedQueueTests(QueueTests):
+
+ queue_factory = SerializedQueue

Don't we need something here? :-)

[17]

Queue should probably use QueueItems too (without delete). That said, since it's just a reference implementation and we'll likely need the reliable version, feel free to ignore this.

[18]

It feels like there's potential for more reuse in the _get_item implementation, but I don't think we should wait this for merging. It's just a good refactoring for a boring moment.