Merging forces

[kragniz]

Merging forces

[lavagetto]

Ok so, I compared the two code bases and the first thing I noticed is that I used the grpc.beta implementation of grpc, while you used the standard one. The reason to use the beta implementation is simply that the non-beta one doesn't support per-request timeouts, which IMO are pretty necessary for the library to be used in a production environment.

I took a look at your code and I think the changes to use the beta implementation would be pretty limited, would you be interested in supporting request-specific timeouts? If so, my plan for adding features would be something like what follows:

• Switch to the beta implementation, add support for per-request timeouts
• Add TLS support
• Add a error handling wrapper for requests
• Add the ability to connect to a different server if the current one fails

[kragniz]

That sounds like a good list of additions.

[lavagetto]

@SamuelMarks I first started going down the contextmanager way, but that comes from a misconception about what a transaction is in etcd v3 terms:

a Transaction is a request where if some conditions are met then action1 is taken, and on failure some others are; but it's a single request, so there is no way for python to know if the precondition works as you tried to suggest.

I think even implementing the format @kragniz was proposing might be partially challenging
(specifically, the etcd.transaction.value('/foo') == 'a' part might be hard to implement correctly)
but that format respects the way a transaction is going to happen.

[kragniz]

Hey, thanks for the interest. Of course, pull requests are always welcome!

What does the transaction-related syntactic sugar look like? My current plan for transactions is to have a slightly verbose, but powerful method containing a list of comparisons, success actions and failure actions:

etcd.transaction(
    compare=[
        etcd.transactions.value('/doot/testing') == 'doot',
        etcd.transactions.version('/doot/testing') > 0,
    ],
    success=[
        etcd.transactions.put('/doot/testing', 'success'),
    ],
    failure=[
        etcd.transactions.put('/doot/testing', 'failure'),
    ]
)

and then writing common transactions in terms of this method (eg put_if_equal, delete_if_equal)

I'm both glad and sad to hear that it's not just me failing to implement watch correctly (my initial attempt is here: 28990ee)

[lavagetto]

@kragniz my implementation is not very different from yours, but maybe less elegant. Still, it kind-of reminds me of interfaces I'm used to and it's pretty easy to use.

        txn = EtcdTransaction()
        txn.when('/test', 'value == "test"').when(
            '/test', 'created_at == NULL' # tests key exists
        ).on_success(
            txn.write('/test', 'test1')
        ).on_failure(
            txn.read('/test')
        ).run(client)

I'll start slowly integrating the parts I think fit better, esp. the connection part.

It might break the current API of the client, though.

[kragniz]

I'd personally rather the transaction happened in a single method call on the client without having to create additional objects. I'm trying to keep the API as clean and readable as possible.

We're currently very early in development, so breaking the API is totally okay!

[SamuelMarks]

Why not use a Context manager:

with etcd.transaction() as t:
    t.compare([
        t.value('/doot/testing') == 'doot',
        t.version('/doot/testing') > 0,
    ]).success([
        t.put('/doot/testing', 'success'),
    ],
    failure=[
        t.put('/doot/testing', 'failure'),
    ])
)

Or even more Pythonic

with etcd.transaction() as t:
    with t.compare(t.value(/doot/testing') == 'doot' or t.version('/doot/testing') > 0) as worked:
        if worked: t.put('/doot/testing', 'success')
        else: t.put('/doot/testing', 'failure')
)

With generators you can go even further with that last example, and lazily construct the equivalent to your example (one transaction) but with the more expected design.

[lavagetto]

@kragniz would you think maintaining, to the extent where it's possible, some compatibility of the API to python-etcd a valuable proposition?

I think so, if only for selfish reasons (I don't want to rewrite all of my current code from scratch).

[kragniz]

@lavagetto transactions in the current form work fine, but are incomplete (the only ops implemented are put and get). You can see it in use in the implementation for replace.

I'd be cautious of maintaining API compatibility with python-etcd. Since v3 has a bunch of differences from v2, people are going to have to make modifications to their code anyway, so it's a good place to get a clean API in place before people start depending on it. Perhaps a better solution for you would be to create a facade library that wraps python-etcd3 with a backwards-compatible API for python-etcd. This would help you and others to migrate a little easier.

[vhosakot]

Add TLS support

@lavagetto @kragniz is etcd3's TLS supported?

http://python-etcd3.readthedocs.io/en/latest/search.html?q=cert

I see in http://python-etcd3.readthedocs.io/en/latest/usage.html?highlight=cert#api that we can pass cert information to the etcd3 client as follows:

class etcd3.Etcd3Client(host='localhost', port=2379, ca_cert=None, cert_key=None,
cert_cert=None, timeout=None, user=None, password=None)

Does this mean we can use the etcd3 client to securely interact with an etcd server with TLS enabled? Can you confirm?

[vhosakot]

@lavagetto @kragniz can you confirm if TLS works?

[tinder-tylerport]

still waiting....