Final etcd 0.2.0 support

[lavagetto]

I had to change a few things, given the different structure of the actual server response.

I created the client.children generator that can parse any directory response, even recursive.

What is still missing is support for lock and leader election primitives; after that, we can release the first client to be completely v2 compliant :)

[dsoprea]

The client is probably completely broken, if write() doesn't work:

Python:

>>> c.write('/test_value', 1)

results in this response:

{u'action': u'set', u'node': {u'createdIndex': 7, u'modifiedIndex': 7, u'value': u'1', u'key': u'/test_value'}}

where "node" is unexpected:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/dustin/build/python-etcd/src/etcd/client.py", line 243, in write
    return self._result_from_response(response)
  File "/Users/dustin/build/python-etcd/src/etcd/client.py", line 431, in _result_from_response
    'Unable to decode server response: %s' % e)
etcd.EtcdException: Unable to decode server response: __new__() got an unexpected keyword argument 'node'

Currently allowed keys (which probably don't even need to be mentioned):

    [
        'action',
        'key',
        'value',
        'expiration',
        'ttl',
        'modifiedIndex',
        'prevValue',
        'newKey',
        'dir',
        'kvs'
    ]

There's also a problem with not catching 404s due to non-absolute paths:

>>> c.write('test_value_2', 2)
Error:
404 page not found

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/dustin/build/python-etcd/src/etcd/client.py", line 242, in write
    response = self.api_execute(path, self._MPUT, params)
  File "/Users/dustin/build/python-etcd/src/etcd/client.py", line 480, in api_execute
    etcd.EtcdError.handle(**json.loads(response.data.decode('utf-8')))
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/json/__init__.py", line 338, in loads
    return _default_decoder.decode(s)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/json/decoder.py", line 368, in decode
    raise ValueError(errmsg("Extra data", s, end, len(s)))
ValueError: Extra data: line 1 column 5 - line 2 column 1 (char 4 - 19)

Obviously, it's probably better to enforce '/' as the first character rather than catching 404s.

[lavagetto]

@dsoprea if you use my branch (or this pull request) write() is not broken; what is actually on master works only with v0.2.0-rc1. I can't say I'm happy with versioning in etcd...

About 404s: we may as well catch if a path does not begin with a / and raise an error before performing the request.

Can you open a separate issue on that?

[jplana]

@lavagetto The PR looks good to me. Just a couple of details I miss in it:

When executing the tests we get a couple of warnings due to the changed parameters in the server:

[deprecated] use -ca-file, not -clientCAFile

Would you change those calls?

Another thing I'm missing is some str implementation in EtcdResult so we can do:

>>> import etcd
>>> client = etcd.Client()
>>> r = client.write('/nodes/n1', 1)
>>> print r
# The actual result would be something like 
<etcd.EtcdResult object at 0x101f85190>
# A better output would be:
({'newKey': None, '_children': [], 'createdIndex': 7, 'modifiedIndex': 7, 'value': u'1', 'expiration': None, 'key': u'/nodes/n1', 'ttl': None, 'action': u'set', 'dir': None, 'prevValue': None})

Probably something like

   def __str__(self):
        return "(%r)" % (self.__dict__)

Would be enough, right?

That would let new library users initially "play" with etc from the ipython/python console in a easier way. I'm of course open to suggestions, we could just merge this PR without further ado and commit these "cosmetic" changes later.

As for @dsoprea problem, AFAIK we can't really know from the client side which 'real' version of the server are we talking to.

When I do something like:

$ curl http://127.0.0.1:7001/version
2

even when using other rc's. Probably it's a fair request for upstream, get some real server versioning either in the /version endpoint or in a header for every request (which would be even better as we wouldn't need an additional request and we would't have to deal with an avalanche of version requests when new clients are started).

On the 404 problem, I'd do both, force "/" AND catch the exception returning something meaningful to the final user, just to be on the safe side.

[dsoprea]

I'll use the pull-request, as I don't seem to see a branch that seems to be
yours.

Would you like some help? It seems like there are things in etcd changing
left and right that aren't hardly documented, not to mention supported by
third-parties.

Dustin

On Mon, Dec 30, 2013 at 2:01 AM, Giuseppe Lavagetto <
notifications@github.com> wrote:

@dsoprea https://github.com/dsoprea if you use my branch (or this pull
request) write() is not broken; what is actually on master works only with
v0.2.0-rc1. I can't say I'm happy with versioning in etcd...

About 404s: we may as well catch if a path does not begin with a / and
raise an error before performing the request.

Can you open a separate issue on that?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/14#issuecomment-31335489
.

[lavagetto]

@jplana your comments are perfectly sound, I'll wrap something up this evening, and update the pull request.

[jimrollenhagen]

I've started working on the lock primitive, based off this branch. Would love some feedback on the API here: #17.

[lavagetto]

I fixed the few issues we had. @jplana I think this is mergeable now. We still need to remove "prevValue" as it's been dropped, but I'll just open an issue for that after everything is merged.

[jplana]

Looks good to me. Merging.