python-etcd
python-etcd copied to clipboard
Update DNS discovery to better match ETCD documentation.
Hi,
I've made a little change to better match the documentation available here. It will now search for __etcd-client.tcp.domain instead of __etcd.tcp.domain. I did not had time to implement searching for _etcd-client-ssl first and if there's an error, search for _etcd-client although I've exposed a flag to use it if people want to.
Thanks for the work.
Sebastien
Coverage decreased (-0.4%) to 87.855% when pulling 209661318fa817ceb6409d9d66e3f5eb1c93d049 on sebastienc:dns_discovery into 18c75196ffd04b3104f3ba44356ec1fcbcc4d09a on jplana:master.
Alright, I'll make the changes.
On Sat, Jun 17, 2017 at 06:12 Giuseppe Lavagetto [email protected] wrote:
@lavagetto requested changes on this pull request.
Thanks for the patch, but I think it needs some restructuring.
In src/etcd/client.py https://github.com/jplana/python-etcd/pull/242#discussion_r122565702:
@@ -65,7 +65,8 @@ def init( use_proxies=False, expected_cluster_id=None, per_host_pool_size=10,
lock_prefix="/_locks"
lock_prefix="/_locks",srv_use_ssl=Falsewe already have 'protocol' that can be used instead of a new parameter
In src/etcd/client.py https://github.com/jplana/python-etcd/pull/242#discussion_r122565748:
@@ -223,8 +226,11 @@ def _set_version_info(self): self._version = version_info['etcdserver'] self._cluster_version = version_info['etcdcluster']
- def _discover(self, domain):
srv_name = "_etcd._tcp.{}".format(domain)
- def _discover(self, domain, use_ssl=False):
I don't like this change as it breaks compatibility with the previous version.
I would make discovery first search for the new alias, then fallback to the old one if discovery returns nothing.
Historically we had client SRV support before the official client, and we went with the same key that confd is using. I still see a lot of people might want to preserve that old name. It should be fairly easy to implement too.
Also, since we save protocol in self.protocol, use that instead of adding a parameter.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jplana/python-etcd/pull/242#pullrequestreview-44697511, or mute the thread https://github.com/notifications/unsubscribe-auth/ABe084qubhqHSFp542fj7G-oDvbuOf4Fks5sE6ZzgaJpZM4NrsxQ .
Coverage decreased (-0.2%) to 87.905% when pulling 7e859ab4bb194a8f5d9cc2f1bff6b8241bf2e36d on sebastienc:dns_discovery into 001d85ebdebd12317a999a70284f1a89a09fea79 on jplana:master.
Hi, is this better?