Add LogDog Python client library.

Add LogDog Python client library.

BUG=

Committed: https://github.com/luci/luci-py/commit/7f4d8f8a3cab4ae5283dfa9edce4b594a5592c47

[dnj, 2016-05-06 21:48:26]

dnj@chromium.org changed reviewers:
+ hinoka@chromium.org, iannucci@chromium.org, martiniss@chromium.org,
nodir@chromium.org

[dnj, 2016-05-06 21:48:26]

PTAL. This is the Python equivalent of the Go stream client library. Ultimately
this will be leveraged by third-party applications and the recipe engine to
engage LogDog and acquire stream handles.

[dnj, 2016-05-10 22:03:58]

dnj@chromium.org changed reviewers:
+ estaab@chromium.org

[nodir, 2016-05-10 22:06:29]

nodir@chromium.org changed reviewers:
+ maruel@chromium.org

[nodir, 2016-05-10 22:06:29]

+maruel first because he need to approve the fs structure

[martiniss, 2016-05-10 22:12:38]

code lgtm

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.py
File client/libs/logdog/stream.py (right):

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:21: class StreamParams(_StreamParamsBase):
This could be generated from a protobuf, right? Or am I crazy, I feel like I've
seen it somewhere.

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:169: Note that this is imperfect, as somethig else
could register stream names
typo

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:359: """Returns: (file): A file-like object for a
single text stream.
Is this correct? I think this was copy pasted.

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:390: """A StreamClient implementation that connects
to a Windows named pipe.
Windows? Why does line 394 say UNIX? Bad copy-paste?

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/tests/st...
File client/libs/logdog/tests/streamname_test.py (right):

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/tests/st...
client/libs/logdog/tests/streamname_test.py:27: self.assertTrue(raised, "Stream
name '%s' did not raise ValueError" % (
you can do `with self.assertRaises(ValueError)` instead?

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/varint.py
File client/libs/logdog/varint.py (right):

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/varint.p...
client/libs/logdog/varint.py:16: w (object): A file-like object to write to.
Must implement write.
"Must implement write" not needed IMO.

[dnj (Google), 2016-05-12 17:54:02]

dnj@google.com changed reviewers:
+ dnj@google.com

[dnj (Google), 2016-05-12 17:54:03]

Thanks for reviewing.

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.py
File client/libs/logdog/stream.py (right):

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:21: class StreamParams(_StreamParamsBase):
On 2016/05/10 22:12:38, martiniss wrote:
> This could be generated from a protobuf, right? Or am I crazy, I feel like
I've
> seen it somewhere.

Nope, this is a Go struct, so we need a Python version.

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:169: Note that this is imperfect, as somethig else
could register stream names
On 2016/05/10 22:12:38, martiniss wrote:
> typo

Done.

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:359: """Returns: (file): A file-like object for a
single text stream.
On 2016/05/10 22:12:38, martiniss wrote:
> Is this correct? I think this was copy pasted.

Yep was copy/pasted, cleaned that up.

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:390: """A StreamClient implementation that connects
to a Windows named pipe.
On 2016/05/10 22:12:38, martiniss wrote:
> Windows? Why does line 394 say UNIX? Bad copy-paste?

Was copy/pasted.

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/tests/st...
File client/libs/logdog/tests/streamname_test.py (right):

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/tests/st...
client/libs/logdog/tests/streamname_test.py:27: self.assertTrue(raised, "Stream
name '%s' did not raise ValueError" % (
On 2016/05/10 22:12:38, martiniss wrote:
> you can do `with self.assertRaises(ValueError)` instead?

Done.

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/varint.py
File client/libs/logdog/varint.py (right):

https://codereview.chromium.org/1961603002/diff/1/client/libs/logdog/varint.p...
client/libs/logdog/varint.py:16: w (object): A file-like object to write to.
Must implement write.
On 2016/05/10 22:12:38, martiniss wrote:
> "Must implement write" not needed IMO.

Well, file-like objects can implement "read" and "write", so I want to be clear
that all I care about here is "write".

[nodir, 2016-05-12 21:29:58]

pure drive-by, didn't review all, feel free to ignore

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/boot...
File client/libs/logdog/bootstrap.py (right):

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/boot...
client/libs/logdog/bootstrap.py:14: pass
Unnecessary because docstring

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/stre...
File client/libs/logdog/stream.py (right):

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/stre...
client/libs/logdog/stream.py:8: import json
Sort

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/stre...
client/libs/logdog/stream.py:23: """
Join lines for consistency? here and below

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/stre...
client/libs/logdog/stream.py:208: def new_connection(self, params):
Document type of |params|

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
File client/libs/logdog/varint.py (right):

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:6: import json
sort

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:13: Writes a varint value to the supplied file-like
object.
make this line the first paragraph of this docstring, this is the important part

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:27: byte = (val & 0x7f)
nit: I think code will be more readable if you use binary numbers than
heximecal, e.g. 0b1111111 here

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:35: return count
consider doing break here, like read_uvarint does

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:51: ValueError if the encoded string has trailing
data.
trailing data is fine (?). Did you mean uvarint was not terminated?

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:58: raise ValueError('UVarint was not terminated.')
remove period

[dnj (Google), 2016-05-16 16:11:12]

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/boot...
File client/libs/logdog/bootstrap.py (right):

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/boot...
client/libs/logdog/bootstrap.py:14: pass
On 2016/05/12 21:29:57, nodir wrote:
> Unnecessary because docstring

Acknowledged.

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/stre...
File client/libs/logdog/stream.py (right):

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/stre...
client/libs/logdog/stream.py:8: import json
On 2016/05/12 21:29:57, nodir wrote:
> Sort

Done.

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/stre...
client/libs/logdog/stream.py:23: """
On 2016/05/12 21:29:57, nodir wrote:
> Join lines for consistency? here and below

Done.

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/stre...
client/libs/logdog/stream.py:208: def new_connection(self, params):
On 2016/05/12 21:29:57, nodir wrote:
> Document type of |params|

Done.

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
File client/libs/logdog/varint.py (right):

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:6: import json
On 2016/05/12 21:29:57, nodir wrote:
> sort

Done.

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:13: Writes a varint value to the supplied file-like
object.
On 2016/05/12 21:29:57, nodir wrote:
> make this line the first paragraph of this docstring, this is the important
part

Done.

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:27: byte = (val & 0x7f)
On 2016/05/12 21:29:57, nodir wrote:
> nit: I think code will be more readable if you use binary numbers than
> heximecal, e.g. 0b1111111 here

Hmm ... I think in hexadecimal, but for single-byte values I can see how this is
more visible. I'll go ahead and do this.

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:35: return count
On 2016/05/12 21:29:57, nodir wrote:
> consider doing break here, like read_uvarint does

Done.

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:51: ValueError if the encoded string has trailing
data.
On 2016/05/12 21:29:57, nodir wrote:
> trailing data is fine (?). Did you mean uvarint was not terminated?

Done.

https://codereview.chromium.org/1961603002/diff/20001/client/libs/logdog/vari...
client/libs/logdog/varint.py:58: raise ValueError('UVarint was not terminated.')
On 2016/05/12 21:29:57, nodir wrote:
> remove period

Done.

[dnj (Google), 2016-05-17 15:03:40]

The CQ bit was checked by dnj@google.com

[dnj (Google), 2016-05-17 15:03:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
martiniss@chromium.org
Link to the patchset: https://codereview.chromium.org/1961603002/#ps40001
(title: "Add tests to PRESUBMIT, Nodir's comments.")

[commit-bot: I haz the power, 2016-05-17 15:03:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1961603002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1961603002/40001

[commit-bot: I haz the power, 2016-05-17 15:06:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-17 15:06:42]

Try jobs failed on following builders:
  Luci-py Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Luci-py%20Presubmit/bui...)

[dnj (Google), 2016-05-17 15:24:32]

The CQ bit was checked by dnj@google.com

[dnj (Google), 2016-05-17 15:24:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
martiniss@chromium.org
Link to the patchset: https://codereview.chromium.org/1961603002/#ps60001
(title: "Remove "client." from package name, make pylint happy.")

[commit-bot: I haz the power, 2016-05-17 15:24:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1961603002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1961603002/60001

[commit-bot: I haz the power, 2016-05-17 15:28:23]

Description was changed from

==========
Add LogDog Python client library.

BUG=
==========

to

==========
Add LogDog Python client library.

BUG=

Committed:
https://github.com/luci/luci-py/commit/7f4d8f8a3cab4ae5283dfa9edce4b594a5592c47
==========

[commit-bot: I haz the power, 2016-05-17 15:28:24]

Committed patchset #4 (id:60001) as
https://github.com/luci/luci-py/commit/7f4d8f8a3cab4ae5283dfa9edce4b594a5592c47