Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add user-agent on requests #295

Merged
merged 7 commits into from
Jan 28, 2022
Merged

fix: add user-agent on requests #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9824024
(WIP) fix: add user-agent on requests
Jan 25, 2022
2450ddc
adjusted tests for download and upload
Jan 25, 2022
141faa3
clean it up, test it right
Jan 25, 2022
29b4ee5
blacken it
Jan 25, 2022
6d75db4
adjust headers
Jan 25, 2022
279802d
lint
Jan 25, 2022
6b9ddfc
match x-goog and user-agent
Jan 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion google/resumable_media/_download.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def __init__(self, media_url, stream=None, start=None, end=None, headers=None):
self.end = end
if headers is None:
headers = {}
self._headers = headers
self._headers = _helpers._base_headers(headers)
self._finished = False
self._retry_strategy = common.RetryStrategy()

Expand Down
8 changes: 8 additions & 0 deletions google/resumable_media/_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import hashlib
import logging
import random
import pkg_resources
import warnings

from google.resumable_media import common
Expand All @@ -45,6 +46,13 @@ def do_nothing():
"""Simple default callback."""


def _base_headers(headers):
version = pkg_resources.get_distribution("google-resumable-media").version
headers["User-Agent"] = "gcloud-python/{}-resumable-media".format(version)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not blindly override headers. At most it should append this to the end of an existing header.

I recommend using setdefault() or similar logic next time.

headers["X-Goog-Api-Client"] = "gcloud-python/{}-resumable-media".format(version)
return headers


def header_required(response, name, get_headers, callback=do_nothing):
"""Checks that a specific header is in a headers dictionary.

Expand Down
2 changes: 1 addition & 1 deletion google/resumable_media/_upload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def __init__(self, upload_url, headers=None):
self.upload_url = upload_url
if headers is None:
headers = {}
self._headers = headers
self._headers = _helpers._base_headers(headers)
self._finished = False
self._retry_strategy = common.RetryStrategy()

Expand Down
22 changes: 11 additions & 11 deletions tests/unit/requests/test_download.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from google.resumable_media import _helpers
from google.resumable_media.requests import download as download_mod
from google.resumable_media.requests import _request_helpers

from google.resumable_media._helpers import _base_headers

URL_PREFIX = "https://www.googleapis.com/download/storage/v1/b/{BUCKET}/o/"
EXAMPLE_URL = URL_PREFIX + "{OBJECT}?alt=media"
Expand Down Expand Up @@ -241,7 +241,7 @@ def test_consume_with_stream_hash_check_fail(self, checksum):

assert stream.getvalue() == b"".join(chunks)
assert download.finished
assert download._headers == {}
assert download._headers == _base_headers({})

error = exc_info.value
assert error.response is transport.request.return_value
Expand All @@ -260,7 +260,7 @@ def test_consume_with_stream_hash_check_fail(self, checksum):
"GET",
EXAMPLE_URL,
data=None,
headers={},
headers=_base_headers({}),
stream=True,
timeout=EXPECTED_TIMEOUT,
)
Expand All @@ -271,7 +271,7 @@ def test_consume_with_headers(self):
self._consume_helper(end=end, headers=headers)
range_bytes = "bytes={:d}-{:d}".format(0, end)
# Make sure the headers have been modified.
assert headers == {"range": range_bytes}
assert headers == _base_headers({"range": range_bytes})


class TestRawDownload(object):
Expand Down Expand Up @@ -494,7 +494,7 @@ def test_consume_with_stream_hash_check_fail(self, checksum):

assert stream.getvalue() == b"".join(chunks)
assert download.finished
assert download._headers == {}
assert download._headers == _base_headers({})

error = exc_info.value
assert error.response is transport.request.return_value
Expand All @@ -513,7 +513,7 @@ def test_consume_with_stream_hash_check_fail(self, checksum):
"GET",
EXAMPLE_URL,
data=None,
headers={},
headers=_base_headers({}),
stream=True,
timeout=EXPECTED_TIMEOUT,
)
Expand All @@ -524,7 +524,7 @@ def test_consume_with_headers(self):
self._consume_helper(end=end, headers=headers)
range_bytes = "bytes={:d}-{:d}".format(0, end)
# Make sure the headers have been modified.
assert headers == {"range": range_bytes}
assert headers == _base_headers({"range": range_bytes})


class TestChunkedDownload(object):
Expand Down Expand Up @@ -589,7 +589,7 @@ def test_consume_next_chunk(self):
ret_val = download.consume_next_chunk(transport)
assert ret_val is transport.request.return_value
range_bytes = "bytes={:d}-{:d}".format(start, start + chunk_size - 1)
download_headers = {"range": range_bytes}
download_headers = _base_headers({"range": range_bytes})
transport.request.assert_called_once_with(
"GET",
EXAMPLE_URL,
Expand Down Expand Up @@ -618,7 +618,7 @@ def test_consume_next_chunk_with_custom_timeout(self):
download.consume_next_chunk(transport, timeout=14.7)

range_bytes = "bytes={:d}-{:d}".format(start, start + chunk_size - 1)
download_headers = {"range": range_bytes}
download_headers = _base_headers({"range": range_bytes})
transport.request.assert_called_once_with(
"GET",
EXAMPLE_URL,
Expand Down Expand Up @@ -695,7 +695,7 @@ def test_consume_next_chunk(self):
"GET",
EXAMPLE_URL,
data=None,
headers=download_headers,
headers=_base_headers(download_headers),
stream=True,
timeout=EXPECTED_TIMEOUT,
)
Expand Down Expand Up @@ -725,7 +725,7 @@ def test_consume_next_chunk_with_custom_timeout(self):
"GET",
EXAMPLE_URL,
data=None,
headers=download_headers,
headers=_base_headers(download_headers),
stream=True,
timeout=14.7,
)
Expand Down
13 changes: 7 additions & 6 deletions tests/unit/requests/test_upload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import mock

import google.resumable_media.requests.upload as upload_mod
from google.resumable_media._helpers import _base_headers


URL_PREFIX = "https://www.googleapis.com/upload/storage/v1/b/{BUCKET}/o"
Expand Down Expand Up @@ -48,7 +49,7 @@ def test_transmit(self):
"POST",
SIMPLE_URL,
data=data,
headers=upload_headers,
headers=_base_headers(upload_headers),
timeout=EXPECTED_TIMEOUT,
)
assert upload.finished
Expand All @@ -67,7 +68,7 @@ def test_transmit_w_custom_timeout(self):
"POST",
SIMPLE_URL,
data=data,
headers=expected_headers,
headers=_base_headers(expected_headers),
timeout=12.6,
)

Expand Down Expand Up @@ -103,7 +104,7 @@ def test_transmit(self, mock_get_boundary):
"POST",
MULTIPART_URL,
data=expected_payload,
headers=upload_headers,
headers=_base_headers(upload_headers),
timeout=EXPECTED_TIMEOUT,
)
assert upload.finished
Expand Down Expand Up @@ -141,7 +142,7 @@ def test_transmit_w_custom_timeout(self, mock_get_boundary):
"POST",
MULTIPART_URL,
data=expected_payload,
headers=upload_headers,
headers=_base_headers(upload_headers),
timeout=12.6,
)
assert upload.finished
Expand Down Expand Up @@ -187,7 +188,7 @@ def test_initiate(self):
"POST",
RESUMABLE_URL,
data=json_bytes,
headers=expected_headers,
headers=_base_headers(expected_headers),
timeout=EXPECTED_TIMEOUT,
)

Expand Down Expand Up @@ -223,7 +224,7 @@ def test_initiate_w_custom_timeout(self):
"POST",
RESUMABLE_URL,
data=json_bytes,
headers=expected_headers,
headers=_base_headers(expected_headers),
timeout=12.6,
)

Expand Down
24 changes: 12 additions & 12 deletions tests/unit/test__download.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

from google.resumable_media import _download
from google.resumable_media import common

from google.resumable_media._helpers import _base_headers

EXAMPLE_URL = (
"https://www.googleapis.com/download/storage/v1/b/{BUCKET}/o/{OBJECT}?alt=media"
Expand All @@ -34,7 +34,7 @@ def test_constructor_defaults(self):
assert download._stream is None
assert download.start is None
assert download.end is None
assert download._headers == {}
assert download._headers == _base_headers({})
assert not download._finished
_check_retry_strategy(download)

Expand All @@ -53,7 +53,7 @@ def test_constructor_explicit(self):
assert download._stream is mock.sentinel.stream
assert download.start == start
assert download.end == end
assert download._headers is headers
assert download._headers == _base_headers(headers)
assert not download._finished
_check_retry_strategy(download)

Expand Down Expand Up @@ -102,14 +102,14 @@ def test__prepare_request(self):
assert method1 == "GET"
assert url1 == EXAMPLE_URL
assert payload1 is None
assert headers1 == {}
assert headers1 == _base_headers({})

download2 = _download.Download(EXAMPLE_URL, start=53)
method2, url2, payload2, headers2 = download2._prepare_request()
assert method2 == "GET"
assert url2 == EXAMPLE_URL
assert payload2 is None
assert headers2 == {"range": "bytes=53-"}
assert headers2 == _base_headers({"range": "bytes=53-"})

def test__prepare_request_with_headers(self):
headers = {"spoonge": "borb"}
Expand All @@ -118,8 +118,9 @@ def test__prepare_request_with_headers(self):
assert method == "GET"
assert url == EXAMPLE_URL
assert payload is None
assert new_headers is headers
assert headers == {"range": "bytes=11-111", "spoonge": "borb"}
assert new_headers == _base_headers(
{"range": "bytes=11-111", "spoonge": "borb"}
)

def test__process_response(self):
download = _download.Download(EXAMPLE_URL)
Expand Down Expand Up @@ -171,7 +172,7 @@ def test_constructor_defaults(self):
assert download.chunk_size == chunk_size
assert download.start == 0
assert download.end is None
assert download._headers == {}
assert download._headers == _base_headers({})
assert not download._finished
_check_retry_strategy(download)
assert download._stream is stream
Expand Down Expand Up @@ -288,7 +289,7 @@ def test__prepare_request(self):
assert method1 == "GET"
assert url1 == EXAMPLE_URL
assert payload1 is None
assert headers1 == {"range": "bytes=0-2047"}
assert headers1 == _base_headers({"range": "bytes=0-2047"})

download2 = _download.ChunkedDownload(
EXAMPLE_URL, chunk_size, None, start=19991
Expand All @@ -298,7 +299,7 @@ def test__prepare_request(self):
assert method2 == "GET"
assert url2 == EXAMPLE_URL
assert payload2 is None
assert headers2 == {"range": "bytes=19991-20100"}
assert headers2 == _base_headers({"range": "bytes=19991-20100"})

def test__prepare_request_with_headers(self):
chunk_size = 2048
Expand All @@ -310,9 +311,8 @@ def test__prepare_request_with_headers(self):
assert method == "GET"
assert url == EXAMPLE_URL
assert payload is None
assert new_headers is headers
expected = {"patrizio": "Starf-ish", "range": "bytes=0-2047"}
assert headers == expected
assert new_headers == _base_headers(expected)

def test__make_invalid(self):
download = _download.ChunkedDownload(EXAMPLE_URL, 512, None)
Expand Down
Loading