Streaming: strip unused features

[tseaver]

Uses #1208 as a base.

• Trim / normailze / document imports.
• Rename functions / methods to match PEP 8.
• Make constants / methods used outside their modules public.
• Make functions / methods not imported outside their modules explicitly private.
• Drop unused exceptions.

[dhermes]

Ping me when this is rebased after the other commits get merged.

[tseaver]

@dhermes rebased after #1208 merged.

[dhermes]

Just want to double check that 11 commits is intended after the rebase and we didn't confuse git?

[tseaver]

This is the correct set of commits: I can squash some down, if you think it will help review better.

[dhermes]

No more commits is almost certainly a good thing (easier to review).

[dhermes]

Unfortunately GitHub thinks there are too many changes to gcloud/streaming/test_transfer.py to display.

There are a couple places where you dropped with _Monkey(MUT, http_wrapper=_Dummy()):. Why?

[dhermes]

Also

_Monkey( Request=lambda url, http_method, body: _Request(url, http_method, body) ) 

seems like it could be done with

_Monkey(Request=_Request) 

What am I missing?

[dhermes]

Yay for "Give 'streaming.util' functions PEP8 names."!

[dhermes]

Rut-roh, test_transfer.py is also not visible in https://github.com/tseaver/gcloud-python/commit/2414bfbbe449db06a7a1aaae314dc1d144a24a4b

[dhermes]

Other than getting at the code GitHub won't show me, I've reviewed them all and made my comments.

[tseaver]

There are a couple places where you dropped with _Monkey(MUT, http_wrapper=_Dummy()):. Why?

In that same commit, transfer begins importing names directly, rather than via http_wrapper.<name>.

[tseaver]

But http_wrapper=_Dummy() wasn't actually replacing anything. So why was it there and why was it safe to remove the monkey patch?

Originally, the _Dummy() had the names which transfer expected to find on http_wrapper: over the course (and after I started importing them directly), it "withered away" like the Communist state.

[dhermes]

Ha. OK

[dhermes]

Seems like adding __doc__ to the named tuple and then me reviewing test_transfer.py is what remains. Am I missing anything else?

[tseaver]

(I think) to review the bigger diff:

$ git remote add tseaver https://github.com/tseaver/gcloud-python $ git fetch --all tseaver $ git diff master..tseaver/streaming-strip_unused_features gcloud/streaming/test_transfer.py

[tseaver]

Rebased to fix conflict on gcloud/storage/test_blob.py (from #1222).

[dhermes]

OK the test_transfer.py diff looks fine (I don't know why GitHub wouldn't show it, it was mostly renames)

[dhermes]

Do we have everything else squared away?

[tseaver]

Do we have everything else squared away?

I think so.

[dhermes]

OK. LGTM