FishEye and Crucible

Shorten up some of these long lines as per PEP-8, please

What does an empty one of these look like? Maybe {} or None or ""? If not defining something empty (your _str_ covers up an empty DataObject), maybe an exception or return type to indicate if a decode or encode worked or not?

How about testing a case where an actual class gets created? For example, I get an encoded string of something like ['lifecycle', 'LCState\x00new', 'name', 'str\x00testResource'] when I do a lifecycle adjustment.

How about showing a proper failure so that we can see how this is handled? Maybe something bad comes through AMQP?

Is it possible for bad data to ever make it into the attributes? If so, can you test the failure case?

Im working on some code for the ResourceAgent that has some generic handling of a resource registry client. Its not perfect, but its a start. Ill get it checked in sometime soon, I hope. I keep getting sidetracked.

Although the following tests do test the implementation of IStore using Cassandra as a backend store, these are more like integration tests than unit tests; a failure in any test might reflect an pure coding/implementation error, an problem/incorrect usage of the cassandra api, or a network problem. It would be ideal to test the implementation separate from actually calling out to cassandra, although I'm not sure of how to do that, or if it is possible. The casssndra python library presumably has a set of unit tests to ensure the cassandra protocol is properly implemented without having to call a running cassandra server.

It might be organizationally more appropriate to only test the store service in the ion.services.coi area.

The intended behavior and implementation of query might deserve a little more thought. After using it in real practice (the registry list function) I made some modifications that seemed to make it more generally useful. Very implementation specific and re specific. See the commit history, if interested.

It should be specified in the IStore interface that self.kvs is an attribute provided by the object. 'kvs' is an OK name; I suggest 'backend_store', or maybe 'backend' but not strongly. Whatever the name, it should be a general word that represents the 'backend key value store' concept, and it should be specified in the interface so that all implementations use the same name.

It is convenient but questionable to build in "factory" functions (as classmethods) for a class such as this; it is likely the backend of any store will require an asynchronous start-up procedure before that needs to occur before the actual store can be either instantiated, or considered ready to provide its behavior.
This classmethod is mixing the concepts of object life-cycle/object creation with object behavior, in this case. Defining a separate "Store Factory" that eventually produces a running store instance, could prove to be more flexible/useful.

This should be renamed 'test_get_nonexistent' or 'test_get_fail'. '404' is something that comes from communication protocol; this is a programming interface.

This might be better named StoreTest, since this particular test is testing the store.Store implementation.

Declaring _init_ as part of the interface is [usually] not appropriate; the interface is a place for implementers to find out what functions they can implement and what behavior the functions should have. _init_ is a Python-Language-defined instance method that is always an option for implementers to implement; and the implementation of _init_ can vary between any two implementations of the same interface. An objects _init_ isn't something that provides [specific] behavior and therefore should not be used in an interface to specify an objects behavior.

After developing cas.py and objstore.py using zope.interface, I recommend updating the IStore interface to use and comply with the zope.interface style.
This will separate the idea of defining the actual interface from the provision of an Abstract Base Class. The meaning of the term Abstract Base Class probably needs clarification, as Python 2.6 actually includes a module that provides such a concept/thing, with its own specific (Pythonic) meaning.

It is a necessity to return a Deferred for all implementations of IStore (even if it utilizes a synchronous backend), because the IStore functions declare a Deferred as their return value.
There were (at least) two choices in how to return deferreds here: one is wrapping the backend call in maybeDeferred; the other is calling the backend normally, and returning an already fired deferred object (defer.succed(result) or defer.fail(reason)). maybeDeferred turns out to be a better choice because it will automatically return a failure (errback) if something goes wrong during the backend call, relieving the implementer from thinking about different exceptions to catch and subsequently manually creating case conditions where defer.fail is returned.

Good job.

Please add concise and descriptive docstrings to the classes. This provides the necessary summary view for people to understand this module.
Think of it as a marketing task.

I think a few of the methods should be refactored into smaller methods.

I'm not sure why you use classmethod for load and reference. I know you might want to use it for create as this follows
a factory pattern. There could be a good reason to use it for load and reference, but I don't know it.

Nice use of asserts. This is a good programming practice.

Small nitpick: make this 4 spaces not 8

Not easy to do, might be possible.

Worth a docstring to note that this is the base class for the tests further down the file.

With this commented out, cruft key/value pairs will accumulate in cassandra. Need a way to cleanup after a test.

How does this differ from get_service_instance?

Is this deprecated code then?

Should this be self.failUnlessEqual? Not sure what assertFalse does with two arguments.