Todd Lipcon has posted comments on this change. Change subject: [python] - Enable Session.apply for list of WriteOperations ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/5231/1/python/kudu/client.pyx File python/kudu/client.pyx: Line 1175: op : WriteOperation or list[WriteOperation] is this style considered pythonic? Personally I'm not a fan of the "it could be a list or not" style, since at most you're saving the user a trivial for loop, and potentially confusing them into thinking there is some special transactionality going on... but if it's the usual way that Python does things I'll acquiesce Line 1190: ops : List[WriteOperation] what's the purpose of returning the ops? I thought it's basically unsafe to use them after they've been applied. Line 1197: raise ValueError("Invalid write operation type.") TypeError Line 1199: for wo in op: given that we've coerced 'op' to a list, I think it's better to rename the paramter to 'ops', or create a new local variable of that name, to indicate the final type is a list. -- To view, visit http://gerrit.cloudera.org:8080/5231 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I637e08547e2f710dded8248a18652b575b239cbe Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jordan Birdsell <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
