Todd Lipcon has posted comments on this change.

Change subject: KUDU-1304 - [python] Enable listing of tablet servers
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4328/2/python/kudu/client.pyx
File python/kudu/client.pyx:

PS2, Line 368: ts = TabletServer()
             :             result.append(ts.init(tservers[i]))
I assume this 'init' thing is because it's not possible to do a normal 
__init__(self) style constructor for the wrapper class or somesuch?


PS2, Line 499: Repere
typo


PS2, Line 500: tservers
expand out the abbreviation here 'TabletServers'


PS2, Line 507: init(
does this show up as a method in the generated python class? If so, maybe it 
should be _init since it's essentially only usable "private"?


Line 510: 
I think we need some __dealloc__ here which takes care of deleting _tserver to 
prevent a leak


-- 
To view, visit http://gerrit.cloudera.org:8080/4328
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I51794168750b358d8c1b371ac2bd08b5c8f15395
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell <jordantbirds...@gmail.com>
Gerrit-Reviewer: Jordan Birdsell <jordantbirds...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to