Siblings, and other bugs #243
Conversation
In order to... 1. Better detect deleted objects and sibling tombstones 2. Reduce server roundtrips to fetch object siblings (which was an inconsistency between HTTP and PBC). ...this refactor introduces breaking changes. This introduces the `RiakContent` class which models a single sibling. An `RiakObject` that is NOT in conflict will appear to have one "sibling". Objects that are strictly "not found" will have ZERO siblings. Otherwise, the object will have as many siblings as are returned by Riak. This is more consistent with Riak's model of objects (values). Most existing methods and properties that were on `RiakObject` have been proxied in such a manner that if the object is not in conflict, the corresponding property or method on the solitary sibling will be invoked. When in conflict, these properties and methods with raise the new `riak.ConflictError` exception. Except in the case of conflict, this will ease transition of existing code to the new behavior. The `get_sibling` method and behavior has been broken to reflect the fact that all siblings are always requested and handled appropriately. The method now produces a deprecation warning and simply returns the requested sibling already in the object. The HTTP transport no longer supports retrieving the "text" format of objects in conflict. This commit also completes a refactoring of the HTTP transport to break-out transport codec methods into a separate class/file, for clarity and consistency with the PBC transport.
This addresses the possibility that an object might be fetched from one interface/transport and stored in the other. Instead, we wrap the raw value in an object that can handle decoding and encoding the format that the transport requires.
| if robj.key is None: | ||
| response = self._request('POST', url, headers, content) | ||
| expect = [201] |
There was a problem hiding this comment.
Isn't 200 also a valid return status if return_body = True?
Also does idiomatic python suggest that we should use a tuple instead of a list? (This is probably not that important, however)
There was a problem hiding this comment.
Yes, we should test the return_body parameter here and add 200 to the list.
I'm not sure about which is more idiomatic; to me, tuples seem most appropriate when position matters, but here it doesn't. Is there some performance/garbage-related reason to use tuples?
There was a problem hiding this comment.
Reviewing the code again, the return_body / 200 case is already handled.
|
So, thanks to @javajolt and @reiddraper I have a plan for how to add sibling-resolution functions to the mix, but I'm not sure this is the appropriate PR to do it in. Thoughts? |
|
Seems like there is already plenty going on in this PR. Maybe the sibling-resolution stuff should be broken out? That said, I'm sure it would be dependent upon this one. |
|
👍 for the change. Proposed roughly the same method under my riak-python-client2 experiment. When can we see this merged? |
It returns a bare tuple, not a named RiakLink tuple.
|
@javajolt Added a test for tombstones that exist as siblings. It turns out that when resolving values internally, Riak will collapse all tombstones into a single one. |
|
Latest commits look good. 👍 |
Three commits, huge and IMO beneficial changes, messages repeated here for your benefit. What can I say, a long plane ride is great for coding.
Refactor model and handling of siblings.
In order to...
...this refactor introduces breaking changes.
This introduces the
RiakContentclass which models a single sibling. AnRiakObjectthat is NOT in conflict will appear to have one "sibling". Objects that are strictly "not found" will have ZERO siblings. Otherwise, the object will have as many siblings as are returned by Riak. This is more consistent with Riak's model of objects (values).Most existing methods and properties that were on
RiakObjecthave been proxied in such a manner that if the object is not in conflict, the corresponding property or method on the solitary sibling will be invoked. When in conflict, these properties and methods with raise the newriak.ConflictErrorexception. Except in the case of conflict, this will ease transition of existing code to the new behavior.The
get_siblingmethod and behavior has been broken to reflect the fact that all siblings are always requested and handled appropriately. The method now produces a deprecation warning and simply returns the requested sibling already in the object. The HTTP transport no longer supports retrieving the "text" format of objects in conflict.This commit also completes a refactoring of the HTTP transport to break-out transport codec methods into a separate class/file, for clarity and consistency with the PBC transport.
Correct usage and handling of last_modified metadata.
HTTP and PBC both handled the "Last-Modified" metadatum differently, and
RiakObject(nowRiakContent) did not expose it properly. Now both transports convert the metadatum into a floating-point UNIX timestamp (UTC). Additionally, the "ETag" metadatum is now consistently named and exposed.Normalize handling of vector clocks across transports.
This addresses the possibility that an object might be fetched from one interface/transport and stored in the other. Instead, we wrap the raw value in an object that can handle decoding and encoding the format that the transport requires.
Unfinished business
put_newmethod, since it is equivalent toputnow.RiakLinkwhen decoding links or just plain tuples, normalizeRiakObject.exists, when in conflict it returnsTrue