[kudu-CR] [python] - Enable Session.apply for list of WriteOperations

["Todd Lipcon (Code Review)" <ge...@cloudera.org>]

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 <jt...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes