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/09/15 00:11:33 UTC

[kudu-CR] [python] Enable Set/Get of unixtime micros and setting of read mode

Jordan Birdsell has uploaded a new change for review.

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

Change subject: [python] Enable Set/Get of unixtime_micros and setting of read mode
......................................................................

[python] Enable Set/Get of unixtime_micros and setting of read mode

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type.  Additionally, it
does not support setting the read mode (for snapshot reading). This
patch enables both of those capabilities as well as enabling write
operations by column index.  Related JIRAs: KUDU-1614, KUDU-1612,
KUDU-1615

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/__init__.py
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
M python/kudu/util.py
8 files changed, 336 insertions(+), 17 deletions(-)


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

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

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins,

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

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

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

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................

KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
A python/kudu/tests/util.py
M python/kudu/util.py
M python/requirements.txt
M python/setup.py
10 files changed, 268 insertions(+), 64 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4417/7
-- 
To view, visit http://gerrit.cloudera.org:8080/4417
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 7
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-1614 - [python] Enable Set/Get of unixtime micros

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 8: Code-Review+1

Leaving a +1 so that Todd can take one last look

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 8
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: No

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/client.pyx
File python/kudu/client.pyx:

PS3, Line 1743: # Leave it to 
> Nit: extra whitespace here and the next two lines.
Done


http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/libkudu_client.pxd
File python/kudu/libkudu_client.pxd:

PS3, Line 217: Status GetInt32(Slice& col_name, int3
> Nit: could you do me a favor and reorganize these so either all the name-ba
Done


http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/tests/common.py
File python/kudu/tests/common.py:

PS3, Line 167: 'unixtime_micro
> Nit: rename to unixtime_micros_val or something. There may eventually be a 
Done


http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/tests/test_scantoken.py
File python/kudu/tests/test_scantoken.py:

PS3, Line 173: 
> Is there a test_util.py or something where this duplicated block of code co
Done


http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/util.py
File python/kudu/util.py:

PS3, Line 75: if isinstance(unixtime_micros, int):
> Missing else?
oops, done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
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: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 9: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 9
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: No

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 4: Code-Review+1

LGTM but this is one of my first times looking at the Python code so I will leave the +2 to Mr. Alves.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
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: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

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/4417

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

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................

KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
M python/kudu/util.py
7 files changed, 191 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4417/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4417
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 3
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

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Reviewed-on: http://gerrit.cloudera.org:8080/4417
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
A python/kudu/tests/util.py
M python/kudu/util.py
M python/requirements.txt
M python/setup.py
10 files changed, 287 insertions(+), 67 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 10
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] [python] Enable Set/Get of unixtime micros and setting of read mode

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: [python] Enable Set/Get of unixtime_micros and setting of read mode
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4417/1//COMMIT_MSG
Commit Message:

Line 11: does not support setting the read mode (for snapshot reading). This
this are two different things likely tested in different tests, can you split this in two patches?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 1
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-HasComments: Yes

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4417/4/python/kudu/util.py
File python/kudu/util.py:

Line 43:     the number of microseconds since the unix epoch
> should clarify the timezone handling. If I wrote "14:34:05" does that mean 
UTC, but good point, added some some checks/conversions as well in case the incoming datetime has a timezone assigned.


Line 64:         raise ValueError("Invalid timestamp type. " +
> what's the range of Python datetime? Is this always possible? Also, are you
Made sure its tz aware, but strictly only returns UTC. As for range, its 1/1/1 0:0:0.000000 to 12/31/9999 23:59:59.999999.

While this doesnt anywhere near cover the full range of int64_t, it does have a pretty wide range. Do you think we need to account for values outside of that range?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 5
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-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-tidy-bot/19/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 3
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-HasComments: No

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/3434/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 3
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-HasComments: No

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 3:

(5 comments)

Looks nice; mostly nits. It's good to be able to use a datetime to set a timestamp-- I think this makes Python client support for the unixtime_micros column exceed that of the Java and C++ clients!

http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/client.pyx
File python/kudu/client.pyx:

PS3, Line 1743:     # Leave it
Nit: extra whitespace here and the next two lines.


http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/libkudu_client.pxd
File python/kudu/libkudu_client.pxd:

PS3, Line 217: Status GetUnixTimeMicros(int col_idx,
Nit: could you do me a favor and reorganize these so either all the name-based getters are together and all the index-based getters are together (same order of types within each group), or each type-specific pair of getters (one name-based, one index-based) is together? Thanks!


http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/tests/common.py
File python/kudu/tests/common.py:

PS3, Line 167: 'timestamp_val'
Nit: rename to unixtime_micros_val or something. There may eventually be a timestamp type again, one that is more aligned with a timestamp type used by Impala or elsewhere in the Hadoopiverse, so may as well avoid the potential collision now.


http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/tests/test_scantoken.py
File python/kudu/tests/test_scantoken.py:

PS3, Line 173: # Insert new rows
Is there a test_util.py or something where this duplicated block of code could be shared by the scanner and scan token tests?


http://gerrit.cloudera.org:8080/#/c/4417/3/python/kudu/util.py
File python/kudu/util.py:

PS3, Line 75: if isinstance(unixtime_micros, int):
Missing else?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 3
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: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4417/8/python/kudu/client.pyx
File python/kudu/client.pyx:

Line 1772:         elif t == KUDU_DOUBLE:
> where are these capabilities documented? maybe PartialRow should have a pyd
Agreed, so, the pydoc stuff is going to take some rework of the docstrings and some structure of the package after looking through it. Are you thinking we should address it class by class or do the api docs for the whole package at once. If we want to do it all at once, would it make sense to throw together something on the website in asciidoc for some common usage examples?


Line 1779:             slc = new Slice(cpython.PyBytes_AsString(value))
> while we're here, maybe we should add a blanket else: raise Exception("Unab
Done


http://gerrit.cloudera.org:8080/#/c/4417/8/python/kudu/util.py
File python/kudu/util.py:

Line 26:     timezone provided.
> nit: trailing space
Done


Line 68:     if timestamp.tzinfo and timestamp.utcoffset():
> https://docs.python.org/2/library/datetime.html#datetime.datetime.now says 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 9
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-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Will Berkeley, Kudu Jenkins,

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

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

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

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................

KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
A python/kudu/tests/util.py
M python/kudu/util.py
M python/requirements.txt
M python/setup.py
10 files changed, 287 insertions(+), 67 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4417/9
-- 
To view, visit http://gerrit.cloudera.org:8080/4417
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 9
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-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins,

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

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

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

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................

KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
A python/kudu/tests/util.py
M python/kudu/util.py
M python/requirements.txt
M python/setup.py
10 files changed, 268 insertions(+), 64 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4417/6
-- 
To view, visit http://gerrit.cloudera.org:8080/4417
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 6
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-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has uploaded a new patch set (#4).

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................

KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
A python/kudu/tests/util.py
M python/kudu/util.py
8 files changed, 237 insertions(+), 63 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
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: Will Berkeley <wd...@gmail.com>

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins,

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

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

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

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................

KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
A python/kudu/tests/util.py
M python/kudu/util.py
M python/requirements.txt
M python/setup.py
10 files changed, 268 insertions(+), 64 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4417/8
-- 
To view, visit http://gerrit.cloudera.org:8080/4417
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 8
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-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins,

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

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

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

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................

KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
A python/kudu/tests/util.py
M python/kudu/util.py
8 files changed, 266 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/4417/5
-- 
To view, visit http://gerrit.cloudera.org:8080/4417
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 5
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-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-tidy-bot/17/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
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-HasComments: No

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4417/8/python/kudu/client.pyx
File python/kudu/client.pyx:

Line 1772:             #  eg: ("2016-01-01", "%Y-%m-%d")
> Agreed, so, the pydoc stuff is going to take some rework of the docstrings 
Yea, I think having pydoc on the APIs would be good, and additionally a page of textual documentation with some "tutorial" type content would be extra awesome.

Fine to do in separate patches, though.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 8
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] [python] Enable Set/Get of unixtime micros and setting of read mode

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: [python] Enable Set/Get of unixtime_micros and setting of read mode
......................................................................


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/3427/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jordan Birdsell <jo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4417/8/python/kudu/client.pyx
File python/kudu/client.pyx:

Line 1772:             #  eg: ("2016-01-01", "%Y-%m-%d")
where are these capabilities documented? maybe PartialRow should have a pydoc with some example usage, and documentation regarding timestamps in particular since it's non-obvious?


Line 1779:                 to_unixtime_micros(value))
while we're here, maybe we should add a blanket else: raise Exception("Unable to set kudu type <foo>") type thing so we don't have silent errors in the future?

also, this line should be indented a bit


http://gerrit.cloudera.org:8080/#/c/4417/8/python/kudu/util.py
File python/kudu/util.py:

Line 26:     timezone provided. 
nit: trailing space


Line 68:     if timestamp.tzinfo:
https://docs.python.org/2/library/datetime.html#datetime.datetime.now says that a timestamp is considered "naive" unless it has both a tzinfo and utcoffset() returns something other than None. So, I guess this should also check that utcoffset() is not None?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 8
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-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4417/4/python/kudu/util.py
File python/kudu/util.py:

Line 43:       If a string is provided, a format must be provided as well.
should clarify the timezone handling. If I wrote "14:34:05" does that mean in local time or UTC? I'm guessing UTC, but should specify (and test)


Line 64:     Convert the input unixtime_micros value to a datetime.
what's the range of Python datetime? Is this always possible? Also, are you creating a tz-aware datetime? https://docs.python.org/2/library/datetime.html seems to indicate there are lot of variants here and this stuff's tricky.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
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>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Jordan Birdsell (Code Review)" <ge...@cloudera.org>.
Jordan Birdsell has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4417/1//COMMIT_MSG
Commit Message:

Line 11: this capability and includes a unit test. This patch also fixes
> this are two different things likely tested in different tests, can you spl
Heres the first one, i'll post the other soon


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
Gerrit-PatchSet: 3
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-HasComments: Yes

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/3433/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
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-HasComments: No

[kudu-CR] KUDU-1614 - [python] Enable Set/Get of unixtime micros

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/4417

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

Change subject: KUDU-1614 - [python] Enable Set/Get of unixtime_micros
......................................................................

KUDU-1614 - [python] Enable Set/Get of unixtime_micros

Currently, the python client in Kudu does not support setting or
getting columns with the unixtime_micros type. This patch enables
this capability and includes a unit test. This patch also fixes
a minor bug with write operations using column indexes (KUDU-1615).
This fix is reflected in the unit test associated with this patch.

Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
---
M python/kudu/client.pyx
M python/kudu/libkudu_client.pxd
M python/kudu/tests/common.py
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scanner.py
M python/kudu/tests/test_scantoken.py
M python/kudu/util.py
7 files changed, 193 insertions(+), 13 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id428cbd072b7de7a75e58b66e4de89acd381fdca
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