Update LogDog client library to generate URLs.

Update LogDog client library to generate URLs.

Update the LogDog client library to generate LogDog stream viewer URLs.
This includes:
- Capturing the LogDog Coordinator host from the enviornment via
  bootstrap.
- Implementing full stream paths and URL generation.
- Adjusting the StreamClient implementations to retain the prefix and
  coordinator host values.
- Wrapping text/binary StreamClient file descriptors in a container to
  expose per-stream higher-level functionality.

Tests have also been updated to assert the new functionality.

TBR=nodir@chromium.org
BUG=chromium:659291
TEST=unit

Committed: https://github.com/luci/luci-py/commit/a32ab2fad3bda41efae1607ec66be78f48a9d0b9

[dnj, 2016-10-27 06:04:54]

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

[dnj, 2016-10-27 06:04:55]

PTAL

[jbudorick, 2016-10-27 13:24:06]

I'm not at all familiar with this code, so it'd probably be good to have someone
else who is look at it as well. That said, lgtm w/ nits, and thanks for the
rapid turnaround here!

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/bootstra...
File client/libs/logdog/bootstrap.py (right):

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/bootstra...
client/libs/logdog/bootstrap.py:79: reg = reg or stream._default_registry
nit: can this just be `reg or stream` given your change to stream.create?

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

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:163: def path(self):
nit: seems a bit odd to have path be a property and get_viewer_url not be (as
viewer_url) given the similar implementations.

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:261: return streamname.get_logdog_viewer_url(
nit: there's a streamname module and a stream_name parameter here? That seems
confusing.

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/streamna...
File client/libs/logdog/streamname.py (right):

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/streamna...
client/libs/logdog/streamname.py:141: 
nit: missing Args

[dnj, 2016-10-27 22:42:45]

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/bootstra...
File client/libs/logdog/bootstrap.py (right):

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/bootstra...
client/libs/logdog/bootstrap.py:79: reg = reg or stream._default_registry
On 2016/10/27 13:24:06, jbudorick wrote:
> nit: can this just be `reg or stream` given your change to stream.create?

It could, but I'd like the types to be consistent between the two choices, so I
think this is more readable.

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

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:163: def path(self):
On 2016/10/27 13:24:06, jbudorick wrote:
> nit: seems a bit odd to have path be a property and get_viewer_url not be (as
> viewer_url) given the similar implementations.

Agreed. I don't really like it, but OTOH having a property that can raise
exceptions also feels weird.

I'll document the exceptions that this can raise, but I think it should stay a
function.

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:261: return streamname.get_logdog_viewer_url(
On 2016/10/27 13:24:06, jbudorick wrote:
> nit: there's a streamname module and a stream_name parameter here? That seems
> confusing.

Will change to 'name".

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/streamna...
File client/libs/logdog/streamname.py (right):

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/streamna...
client/libs/logdog/streamname.py:141: 
On 2016/10/27 13:24:06, jbudorick wrote:
> nit: missing Args

Done.

[dnj, 2016-10-27 22:42:51]

The CQ bit was checked by dnj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 22:42:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 23:15:39]

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

[commit-bot: I haz the power, 2016-10-27 23:15:40]

Dry run: This issue passed the CQ dry run.

[jbudorick, 2016-10-27 23:42:50]

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/bootstra...
File client/libs/logdog/bootstrap.py (right):

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/bootstra...
client/libs/logdog/bootstrap.py:79: reg = reg or stream._default_registry
On 2016/10/27 22:42:44, dnj wrote:
> On 2016/10/27 13:24:06, jbudorick wrote:
> > nit: can this just be `reg or stream` given your change to stream.create?
> 
> It could, but I'd like the types to be consistent between the two choices, so
I
> think this is more readable.

Fair enough.

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

https://codereview.chromium.org/2453273002/diff/1/client/libs/logdog/stream.p...
client/libs/logdog/stream.py:163: def path(self):
On 2016/10/27 22:42:45, dnj wrote:
> On 2016/10/27 13:24:06, jbudorick wrote:
> > nit: seems a bit odd to have path be a property and get_viewer_url not be
(as
> > viewer_url) given the similar implementations.
> 
> Agreed. I don't really like it, but OTOH having a property that can raise
> exceptions also feels weird.
> 
> I'll document the exceptions that this can raise, but I think it should stay a
> function.

sgtm.

[dnj, 2016-11-02 00:23:52]

Description was changed from

==========
Update LogDog client library to generate URLs.

Update the LogDog client library to generate LogDog stream viewer URLs.
This includes:
- Capturing the LogDog Coordinator host from the enviornment via
  bootstrap.
- Implementing full stream paths and URL generation.
- Adjusting the StreamClient implementations to retain the prefix and
  coordinator host values.
- Wrapping text/binary StreamClient file descriptors in a container to
  expose per-stream higher-level functionality.

Tests have also been updated to assert the new functionality.

BUG=chromium:659291
TEST=unit
==========

to

==========
Update LogDog client library to generate URLs.

Update the LogDog client library to generate LogDog stream viewer URLs.
This includes:
- Capturing the LogDog Coordinator host from the enviornment via
  bootstrap.
- Implementing full stream paths and URL generation.
- Adjusting the StreamClient implementations to retain the prefix and
  coordinator host values.
- Wrapping text/binary StreamClient file descriptors in a container to
  expose per-stream higher-level functionality.

Tests have also been updated to assert the new functionality.

TBR=nodir@chromium.org
BUG=chromium:659291
TEST=unit
==========

[dnj, 2016-11-02 00:23:55]

The CQ bit was checked by dnj@chromium.org

[dnj, 2016-11-02 00:23:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/2453273002/#ps20001
(title: "Forgot project, oops. Addressed nits.")

[commit-bot: I haz the power, 2016-11-02 00:24:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 00:26:40]

Description was changed from

==========
Update LogDog client library to generate URLs.

Update the LogDog client library to generate LogDog stream viewer URLs.
This includes:
- Capturing the LogDog Coordinator host from the enviornment via
  bootstrap.
- Implementing full stream paths and URL generation.
- Adjusting the StreamClient implementations to retain the prefix and
  coordinator host values.
- Wrapping text/binary StreamClient file descriptors in a container to
  expose per-stream higher-level functionality.

Tests have also been updated to assert the new functionality.

TBR=nodir@chromium.org
BUG=chromium:659291
TEST=unit
==========

to

==========
Update LogDog client library to generate URLs.

Update the LogDog client library to generate LogDog stream viewer URLs.
This includes:
- Capturing the LogDog Coordinator host from the enviornment via
  bootstrap.
- Implementing full stream paths and URL generation.
- Adjusting the StreamClient implementations to retain the prefix and
  coordinator host values.
- Wrapping text/binary StreamClient file descriptors in a container to
  expose per-stream higher-level functionality.

Tests have also been updated to assert the new functionality.

TBR=nodir@chromium.org
BUG=chromium:659291
TEST=unit

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

[commit-bot: I haz the power, 2016-11-02 00:26:42]

Committed patchset #2 (id:20001) as
https://github.com/luci/luci-py/commit/a32ab2fad3bda41efae1607ec66be78f48a9d0b9