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

2016-09-22 Thread Todd Lipcon (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

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

2016-09-19 Thread Jordan Birdsell (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

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

2016-09-19 Thread Jordan Birdsell (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

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

2016-09-19 Thread Todd Lipcon (Code Review) 
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 ") 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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

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

2016-09-15 Thread Jordan Birdsell (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

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

2016-09-15 Thread Jordan Birdsell (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley

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

2016-09-15 Thread Todd Lipcon (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

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

2016-09-15 Thread Will Berkeley (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

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

2016-09-15 Thread Jordan Birdsell (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Will Berkeley

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

2016-09-14 Thread Jordan Birdsell (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jordan Birdsell 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

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

2016-09-14 Thread Kudu Jenkins (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

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

2016-09-14 Thread Kudu Jenkins (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

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

2016-09-14 Thread Kudu Jenkins (Code Review) 
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 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No