Will Berkeley has posted comments on this change. Change subject: [python] - Update kudu.connect to enable multi-master ......................................................................
Patch Set 4: (5 comments) I think we can use the approach that was used with single master but do it for each master: set the --server_dump_info_path flag and read the port from there. See my comments on common.py patch + base (I hope commenting on the isn't hard work with...never done it before...sorry if makes it harder) http://gerrit.cloudera.org:8080/#/c/4883/4/python/kudu/__init__.py File python/kudu/__init__.py: PS4, Line 89: addresses What happens if this is empty? http://gerrit.cloudera.org:8080/#/c/4883/4/python/kudu/tests/common.py File python/kudu/tests/common.py: PS4, Line 69: This flag makes the server dump info -- including the port it bound to -- to this file, in json format by default. PS4, Line 88: : Can you open the server info file for each master and get the port there? PS4, Line 61: s = socket.socket() : s.bind(('', 0)) : master_ports.append(s.getsockname()[1 I don't think we need to do this. The code now gets the bound port by reading it from a file the server can be made to create via a flag, and I think we can do this for each master. See related comments below. PS4, Line 64: s.close() The socket module docs mention it's best to call socket.shutdown before socket.close if the connection should be closed ASAP. -- To view, visit http://gerrit.cloudera.org:8080/4883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia2721236d5f92ced2afb4a867511c4144a2ab16a Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jordan Birdsell <jordantbirds...@gmail.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-HasComments: Yes
