You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Jordan Birdsell (Code Review)" <ge...@cloudera.org> on 2016/10/20 00:09:20 UTC

[kudu-CR] KUDU-1680 - [python] Improve PartialRow Usability

Jordan Birdsell has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4760

Change subject: KUDU-1680 - [python] Improve PartialRow Usability
......................................................................

KUDU-1680 - [python] Improve PartialRow Usability

The current semantics for setting values in a PartialRow are not
very elegant or comfortable for Python developers. This patch
improves this API by providing the ability to "load" a PartialRow
from a list, tuple or dictionary.  Some existing tests were modified,
however, some tests will continue to use the old API to ensure
backwards compatibility.

Change-Id: I5c9f57358e5048528398818fc80f32b27531a423
---
A python/kudu/client.pxd
M python/kudu/client.pyx
M python/kudu/compat.py
M python/kudu/schema.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/util.py
9 files changed, 243 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/4760/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5c9f57358e5048528398818fc80f32b27531a423
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell <jo...@gmail.com>

[kudu-CR] KUDU-1680 - [python] Improve PartialRow Usability

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.

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 <jo...@gmail.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jordan Birdsell <jo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1680 - [python] Improve PartialRow Usability

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4760

to look at the new patch set (#4).

Change subject: KUDU-1680 - [python] Improve PartialRow Usability
......................................................................

KUDU-1680 - [python] Improve PartialRow Usability

The current semantics for setting values in a PartialRow are not
very elegant or comfortable for Python developers. This patch
improves this API by providing the ability to "load" a PartialRow
from a list, tuple or dictionary.  Some existing tests were modified,
however, some tests will continue to use the old API to ensure
backwards compatibility.

Change-Id: I5c9f57358e5048528398818fc80f32b27531a423
---
A python/kudu/client.pxd
M python/kudu/client.pyx
M python/kudu/compat.py
M python/kudu/schema.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/util.py
9 files changed, 248 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/4760/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5c9f57358e5048528398818fc80f32b27531a423
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell <jo...@gmail.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jordan Birdsell <jo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-1680 - [python] Improve PartialRow Usability

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4760

to look at the new patch set (#2).

Change subject: KUDU-1680 - [python] Improve PartialRow Usability
......................................................................

KUDU-1680 - [python] Improve PartialRow Usability

The current semantics for setting values in a PartialRow are not
very elegant or comfortable for Python developers. This patch
improves this API by providing the ability to "load" a PartialRow
from a list, tuple or dictionary.  Some existing tests were modified,
however, some tests will continue to use the old API to ensure
backwards compatibility.

Change-Id: I5c9f57358e5048528398818fc80f32b27531a423
---
A python/kudu/client.pxd
M python/kudu/client.pyx
M python/kudu/compat.py
M python/kudu/schema.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/util.py
9 files changed, 248 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/4760/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4760
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5c9f57358e5048528398818fc80f32b27531a423
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell <jo...@gmail.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>