Jordan Birdsell has posted comments on this change. Change subject: KUDU-1680 - [python] Improve PartialRow Usability ......................................................................
Patch Set 2: (13 comments) http://gerrit.cloudera.org:8080/#/c/4760/1/python/kudu/client.pxd File python/kudu/client.pxd: PS1, Line 42: add_to_session > Nit: slight preference for calling this apply, in line with C++ and Java. Actually thats how its called from session.apply(op). I think this method was only ever created so that some of the WriteOperation properties didn't have to be made public. I.e., session.apply(op) calls op.add_to_session Also, this file is only being added so that we can use this Class in the Schema module, there actually is nothing being changed here. In cython, pxd is basically the equivalent of a header. http://gerrit.cloudera.org:8080/#/c/4760/1/python/kudu/client.pyx File python/kudu/client.pyx: PS1, Line 653: PartialRow > spelling Done PS1, Line 653: > is used to initialize/create/setup the insert? Done PS1, Line 653: , > sswapped space and comma " ," -> ", " Done PS1, Line 655: an be either col > They can be a mix of name and index too, right? And both name and index cou Yea, gotta love python :), i cant see why youd ever do that, but you can. clarified PS1, Line 670: . Pa > see above Done PS1, Line 670: rat > see above Done PS1, Line 670: . > see above Done PS1, Line 687: > same 3 comments as insert, upsert Done PS1, Line 704: d=None): > ditto Done PS1, Line 2043: loc(ke > sp Done http://gerrit.cloudera.org:8080/#/c/4760/1/python/kudu/schema.pyx File python/kudu/schema.pyx: PS1, Line 546: a PartialRow will be initialized with values fr > same comments as elsewhere- swapped space and comma, misspelled PartialRow, Done http://gerrit.cloudera.org:8080/#/c/4760/1/python/kudu/tests/test_client.py File python/kudu/tests/test_client.py: PS1, Line 173: {0: 2, : 1: 222, : 2: 'upserted'}) > Could you add an op that uses indexes too? Just as a sanity check. Changed this one to index based since I have other tests uses the column names for dict keys. -- To view, visit http://gerrit.cloudera.org:8080/4760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5c9f57358e5048528398818fc80f32b27531a423 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jordan Birdsell <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jordan Birdsell <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-HasComments: Yes
