Client side implementation of new /content-gs isolate protocol.

Client side implementation of new /content-gs isolate protocol.

Replaces existing /content protocol.

BUG=289670
DO NOT SUBMIT

Abandoned.

[Vadim Sh., 2013-09-26 01:14:19]

It's a work in progress. I'm writing unit tests for IsolateServerGS class now.
It's difficult to test both StorageApi implementations :(

https://codereview.chromium.org/24578004/diff/1/isolateserver.py
File isolateserver.py (right):

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode33
isolateserver.py:33: ENABLE_GS_ISOLATE_API = False
I need your advice on how to deal with conditional switching to new
IsolateServerGS protocol.

Possible solutions:
1) Use global switch, like it is done now. Test both implementations in unit
tests. That way we can at least revert fast, but setting ENABLE_GS_ISOLATE_API =
False.
2) Add command line option to isolate.py, isolateserver.py and run_isolated.py
(--use-gs-api). But then we need to change buildbot side to conditionally enable
it :(
3) Just always use new IsolateServerGS and forget about IsolateServer.

[M-A Ruel, 2013-09-26 01:21:47]

https://codereview.chromium.org/24578004/diff/1/isolateserver.py
File isolateserver.py (right):

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode33
isolateserver.py:33: ENABLE_GS_ISOLATE_API = False
On 2013/09/26 01:14:19, Vadim Sh. wrote:
> I need your advice on how to deal with conditional switching to new
> IsolateServerGS protocol.
> 
> Possible solutions:
> 1) Use global switch, like it is done now. Test both implementations in unit
> tests. That way we can at least revert fast, but setting ENABLE_GS_ISOLATE_API
=
> False.
> 2) Add command line option to isolate.py, isolateserver.py and run_isolated.py
> (--use-gs-api). But then we need to change buildbot side to conditionally
enable
> it :(
> 3) Just always use new IsolateServerGS and forget about IsolateServer.

3)

Let it bake on the canary, then roll deps and monitor.

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode260
isolateserver.py:260: decompressed = decompressor.decompress(chunk)
You should create an inner loop here, so that if a compressed block generates
2gb of data, it doesn't blow python up.

See how the isolate server does it.

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode764
isolateserver.py:764: if "error" in caps:
'error'

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode839
isolateserver.py:839: # Testing seems to show that if a few machines are trying
to download
This likely doesn't apply here.

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode917
isolateserver.py:917: if push_urls is not None
is "if push_urls" sufficient?

[Vadim Sh., 2013-09-27 20:28:11]

This CL is done.
But due to rietveld issues it's in a weird state right now.
I'll abandon this CL and reupload it as a new one, with clean diff.

https://codereview.chromium.org/24578004/diff/1/isolateserver.py
File isolateserver.py (right):

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode33
isolateserver.py:33: ENABLE_GS_ISOLATE_API = False
On 2013/09/26 01:21:47, M-A Ruel wrote:
> On 2013/09/26 01:14:19, Vadim Sh. wrote:
> > I need your advice on how to deal with conditional switching to new
> > IsolateServerGS protocol.
> > 
> > Possible solutions:
> > 1) Use global switch, like it is done now. Test both implementations in unit
> > tests. That way we can at least revert fast, but setting
ENABLE_GS_ISOLATE_API
> =
> > False.
> > 2) Add command line option to isolate.py, isolateserver.py and
run_isolated.py
> > (--use-gs-api). But then we need to change buildbot side to conditionally
> enable
> > it :(
> > 3) Just always use new IsolateServerGS and forget about IsolateServer.
> 
> 3)
> 
> Let it bake on the canary, then roll deps and monitor.

Sweet. It simplifies a lot.

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode260
isolateserver.py:260: decompressed = decompressor.decompress(chunk)
On 2013/09/26 01:21:47, M-A Ruel wrote:
> You should create an inner loop here, so that if a compressed block generates
> 2gb of data, it doesn't blow python up.
> 
> See how the isolate server does it.

Done.

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode764
isolateserver.py:764: if "error" in caps:
On 2013/09/26 01:21:47, M-A Ruel wrote:
> 'error'

Done.

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode839
isolateserver.py:839: # Testing seems to show that if a few machines are trying
to download
On 2013/09/26 01:21:47, M-A Ruel wrote:
> This likely doesn't apply here.

Done.

https://codereview.chromium.org/24578004/diff/1/isolateserver.py#newcode917
isolateserver.py:917: if push_urls is not None
On 2013/09/26 01:21:47, M-A Ruel wrote:
> is "if push_urls" sufficient?

Done.

https://codereview.chromium.org/24578004/diff/20001/isolateserver.py
File isolateserver.py (right):

https://codereview.chromium.org/24578004/diff/20001/isolateserver.py#newcode185
isolateserver.py:185: |content_generator| should yield an entire zipped stream,
this function will
I wrote a test to check that (incomplete stream -> zlib.error). In fact it's not
true :( Zlib can not figure out by itself that stream is truncated.