From 7d889addd0bfa2fd448ca2105ebfe82f617c5133 Mon Sep 17 00:00:00 2001 From: Danny Hermes <daniel.j.hermes@gmail.com> Date: Thu, 20 Jul 2017 15:59:24 -0700 Subject: [PATCH] Dropping usage of "size" to measure (resumable) bytes uploaded. --- google/resumable_media/_upload.py | 16 ++++++++++---- google/resumable_media/requests/upload.py | 2 +- nox.py | 18 +++++++++++---- tests/unit/test__upload.py | 27 +++++++++++------------ 4 files changed, 40 insertions(+), 23 deletions(-) diff --git a/google/resumable_media/_upload.py b/google/resumable_media/_upload.py index 1e2b506b..e3906aa3 100644 --- a/google/resumable_media/_upload.py +++ b/google/resumable_media/_upload.py @@ -548,7 +548,7 @@ def _make_invalid(self): """ self._invalid = True - def _process_response(self, response): + def _process_response(self, response, bytes_sent): """Process the response from an HTTP request. This is everything that must be done after a request that doesn't @@ -557,6 +557,8 @@ def _process_response(self, response): Args: response (object): The HTTP response object. + bytes_sent (int): The number of bytes sent in the request that + ``response`` was returned for. Raises: ~google.resumable_media.common.InvalidResponse: If the status @@ -571,9 +573,15 @@ def _process_response(self, response): response, (http_client.OK, resumable_media.PERMANENT_REDIRECT), self._get_status_code, callback=self._make_invalid) if status_code == http_client.OK: - body = self._get_body(response) - json_response = json.loads(body.decode(u'utf-8')) - self._bytes_uploaded = int(json_response[u'size']) + # NOTE: We use the "local" information of ``bytes_sent`` to update + # ``bytes_uploaded``, but do not verify this against other + # state. However, there may be some other information: + # + # * a ``size`` key in JSON response body + # * the ``total_bytes`` attribute (if set) + # * ``stream.tell()`` (relying on fact that ``initiate()`` + # requires stream to be at the beginning) + self._bytes_uploaded = self._bytes_uploaded + bytes_sent # Tombstone the current upload so it cannot be used again. self._finished = True else: diff --git a/google/resumable_media/requests/upload.py b/google/resumable_media/requests/upload.py index b9442bc6..07642170 100644 --- a/google/resumable_media/requests/upload.py +++ b/google/resumable_media/requests/upload.py @@ -393,7 +393,7 @@ def transmit_next_chunk(self, transport): result = _helpers.http_request( transport, method, url, data=payload, headers=headers, retry_strategy=self._retry_strategy) - self._process_response(result) + self._process_response(result, len(payload)) return result def recover(self, transport): diff --git a/nox.py b/nox.py index f91125a6..8b98e969 100644 --- a/nox.py +++ b/nox.py @@ -42,9 +42,14 @@ def unit_tests(session, python_version): line_coverage = '--cov-fail-under=99' session.run( 'py.test', - '--cov=google.resumable_media', '--cov=tests.unit', '--cov-append', - '--cov-config=.coveragerc', '--cov-report=', line_coverage, + '--cov=google.resumable_media', + '--cov=tests.unit', + '--cov-append', + '--cov-config=.coveragerc', + '--cov-report=', + line_coverage, os.path.join('tests', 'unit'), + *session.posargs ) @@ -102,7 +107,8 @@ def lint(session): session.run( 'flake8', os.path.join('google', 'resumable_media'), - 'tests') + 'tests', + ) @nox.session @@ -140,7 +146,11 @@ def system_tests(session, python_version): session.install('-e', '.') # Run py.test against the system tests. - session.run('py.test', os.path.join('tests', 'system')) + session.run( + 'py.test', + os.path.join('tests', 'system'), + *session.posargs + ) @nox.session diff --git a/tests/unit/test__upload.py b/tests/unit/test__upload.py index 88827a06..9baab335 100644 --- a/tests/unit/test__upload.py +++ b/tests/unit/test__upload.py @@ -557,7 +557,7 @@ def test__process_response_bad_status(self): assert not upload.invalid response = _make_response(status_code=http_client.NOT_FOUND) with pytest.raises(common.InvalidResponse) as exc_info: - upload._process_response(response) + upload._process_response(response, None) error = exc_info.value assert error.response is response @@ -572,16 +572,20 @@ def test__process_response_success(self): upload = _upload.ResumableUpload(RESUMABLE_URL, ONE_MB) _fix_up_virtual(upload) - total_bytes = 158 - response_body = u'{{"size": "{:d}"}}'.format(total_bytes) - response_body = response_body.encode(u'utf-8') - # Check status before. + # Check / set status before. assert upload._bytes_uploaded == 0 + upload._bytes_uploaded = 20 assert not upload._finished + + # Set the response body. + bytes_sent = 158 + total_bytes = upload._bytes_uploaded + bytes_sent + response_body = u'{{"size": "{:d}"}}'.format(total_bytes) + response_body = response_body.encode(u'utf-8') response = mock.Mock( content=response_body, status_code=http_client.OK, spec=[u'content', u'status_code']) - ret_val = upload._process_response(response) + ret_val = upload._process_response(response, bytes_sent) assert ret_val is None # Check status after. assert upload._bytes_uploaded == total_bytes @@ -596,7 +600,7 @@ def test__process_response_partial_no_range(self): # Make sure the upload is valid before the failure. assert not upload.invalid with pytest.raises(common.InvalidResponse) as exc_info: - upload._process_response(response) + upload._process_response(response, None) # Make sure the upload is invalid after the failure. assert upload.invalid @@ -616,7 +620,7 @@ def test__process_response_partial_bad_range(self): response = _make_response( status_code=resumable_media.PERMANENT_REDIRECT, headers=headers) with pytest.raises(common.InvalidResponse) as exc_info: - upload._process_response(response) + upload._process_response(response, 81) # Check the error response. error = exc_info.value @@ -635,7 +639,7 @@ def test__process_response_partial(self): headers = {u'range': u'bytes=0-171'} response = _make_response( status_code=resumable_media.PERMANENT_REDIRECT, headers=headers) - ret_val = upload._process_response(response) + ret_val = upload._process_response(response, 172) assert ret_val is None # Check status after. assert upload._bytes_uploaded == 172 @@ -932,14 +936,9 @@ def _get_headers(response): return response.headers -def _get_body(response): - return response.content - - def _fix_up_virtual(upload): upload._get_status_code = _get_status_code upload._get_headers = _get_headers - upload._get_body = _get_body def _check_retry_strategy(upload):