From 1843878c4b0ac34d09253f7acc55a5aa208b6514 Mon Sep 17 00:00:00 2001 From: Andrew Gorcester Date: Mon, 11 Nov 2024 17:23:23 -0800 Subject: [PATCH] Fix: Remove deprecated num_retries argument --- google/cloud/storage/_helpers.py | 23 +---- google/cloud/storage/blob.py | 112 +--------------------- google/cloud/storage/fileio.py | 15 --- tests/unit/test__helpers.py | 26 ----- tests/unit/test_blob.py | 157 ++----------------------------- tests/unit/test_fileio.py | 128 ------------------------- 6 files changed, 14 insertions(+), 447 deletions(-) diff --git a/google/cloud/storage/_helpers.py b/google/cloud/storage/_helpers.py index e9baa4acf..a4e7c4773 100644 --- a/google/cloud/storage/_helpers.py +++ b/google/cloud/storage/_helpers.py @@ -72,12 +72,6 @@ ("if_source_metageneration_not_match", "ifSourceMetagenerationNotMatch"), ) -_NUM_RETRIES_MESSAGE = ( - "`num_retries` has been deprecated and will be removed in a future " - "release. Use the `retry` argument with a Retry or ConditionalRetryPolicy " - "object, or None, instead." -) - # _NOW() returns the current local date and time. # It is preferred to use timezone-aware datetimes _NOW(_UTC), # which returns the current UTC date and time. @@ -631,7 +625,7 @@ def _bucket_bound_hostname_url(host, scheme=None): return f"{scheme}://{host}" -def _api_core_retry_to_resumable_media_retry(retry, num_retries=None): +def _api_core_retry_to_resumable_media_retry(retry): """Convert google.api.core.Retry to google.cloud.storage._media.RetryStrategy. Custom predicates are not translated. @@ -639,28 +633,17 @@ def _api_core_retry_to_resumable_media_retry(retry, num_retries=None): :type retry: google.api_core.Retry :param retry: (Optional) The google.api_core.Retry object to translate. - :type num_retries: int - :param num_retries: (Optional) The number of retries desired. This is - supported for backwards compatibility and is mutually exclusive with - `retry`. - :rtype: google.cloud.storage._media.RetryStrategy - :returns: A RetryStrategy with all applicable attributes copied from input, - or a RetryStrategy with max_retries set to 0 if None was input. + :returns: A RetryStrategy with all applicable attributes copied from input. """ - if retry is not None and num_retries is not None: - raise ValueError("num_retries and retry arguments are mutually exclusive") - - elif retry is not None: + if retry is not None: return _media.RetryStrategy( max_sleep=retry._maximum, max_cumulative_retry=retry._deadline, initial_delay=retry._initial, multiplier=retry._multiplier, ) - elif num_retries is not None: - return _media.RetryStrategy(max_retries=num_retries) else: return _media.RetryStrategy(max_retries=0) diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index 7bb7d6758..bc837f563 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -59,7 +59,6 @@ from google.cloud.storage._helpers import _get_default_storage_base_url from google.cloud.storage._signing import generate_signed_url_v2 from google.cloud.storage._signing import generate_signed_url_v4 -from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE from google.cloud.storage._helpers import _API_VERSION from google.cloud.storage._helpers import _virtual_hosted_style_base_url from google.cloud.storage.acl import ACL @@ -1829,7 +1828,6 @@ def _do_multipart_upload( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -1866,15 +1864,6 @@ def _do_multipart_upload( ``stream``). If not provided, the upload will be concluded once ``stream`` is exhausted (or :data:`None`). - :type num_retries: int - :param num_retries: - Number of upload retries. By default, only uploads with - if_generation_match set will be retried, as uploads without the - argument are not guaranteed to be idempotent. Setting num_retries - will override this default behavior and guarantee retries even when - if_generation_match is not set. (Deprecated: This argument - will be removed in a future release.) - :type predefined_acl: str :param predefined_acl: (Optional) Predefined access control list @@ -1989,9 +1978,7 @@ def _do_multipart_upload( upload_url = _add_query_parameters(base_url, name_value_pairs) upload = MultipartUpload(upload_url, headers=headers, checksum=checksum) - upload._retry_strategy = _api_core_retry_to_resumable_media_retry( - retry, num_retries - ) + upload._retry_strategy = _api_core_retry_to_resumable_media_retry(retry) response = upload.transmit( transport, data, object_metadata, content_type, timeout=timeout @@ -2005,7 +1992,6 @@ def _initiate_resumable_upload( stream, content_type, size, - num_retries, predefined_acl=None, extra_headers=None, chunk_size=None, @@ -2047,15 +2033,6 @@ def _initiate_resumable_upload( :type predefined_acl: str :param predefined_acl: (Optional) Predefined access control list - :type num_retries: int - :param num_retries: - Number of upload retries. By default, only uploads with - if_generation_match set will be retried, as uploads without the - argument are not guaranteed to be idempotent. Setting num_retries - will override this default behavior and guarantee retries even when - if_generation_match is not set. (Deprecated: This argument - will be removed in a future release.) - :type extra_headers: dict :param extra_headers: (Optional) Extra headers to add to standard headers. @@ -2185,9 +2162,7 @@ def _initiate_resumable_upload( upload_url, chunk_size, headers=headers, checksum=checksum ) - upload._retry_strategy = _api_core_retry_to_resumable_media_retry( - retry, num_retries - ) + upload._retry_strategy = _api_core_retry_to_resumable_media_retry(retry) upload.initiate( transport, @@ -2207,7 +2182,6 @@ def _do_resumable_upload( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -2247,15 +2221,6 @@ def _do_resumable_upload( ``stream``). If not provided, the upload will be concluded once ``stream`` is exhausted (or :data:`None`). - :type num_retries: int - :param num_retries: - Number of upload retries. By default, only uploads with - if_generation_match set will be retried, as uploads without the - argument are not guaranteed to be idempotent. Setting num_retries - will override this default behavior and guarantee retries even when - if_generation_match is not set. (Deprecated: This argument - will be removed in a future release.) - :type predefined_acl: str :param predefined_acl: (Optional) Predefined access control list @@ -2320,7 +2285,6 @@ def _do_resumable_upload( stream, content_type, size, - num_retries, predefined_acl=predefined_acl, if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, @@ -2346,7 +2310,6 @@ def _do_upload( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -2387,15 +2350,6 @@ def _do_upload( ``stream``). If not provided, the upload will be concluded once ``stream`` is exhausted (or :data:`None`). - :type num_retries: int - :param num_retries: - Number of upload retries. By default, only uploads with - if_generation_match set will be retried, as uploads without the - argument are not guaranteed to be idempotent. Setting num_retries - will override this default behavior and guarantee retries even when - if_generation_match is not set. (Deprecated: This argument - will be removed in a future release.) - :type predefined_acl: str :param predefined_acl: (Optional) Predefined access control list @@ -2485,7 +2439,6 @@ def _do_upload( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -2502,7 +2455,6 @@ def _do_upload( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -2522,7 +2474,6 @@ def _prep_and_do_upload( rewind=False, size=None, content_type=None, - num_retries=None, client=None, predefined_acl=None, if_generation_match=None, @@ -2580,15 +2531,6 @@ def _prep_and_do_upload( :type content_type: str :param content_type: (Optional) Type of content being uploaded. - :type num_retries: int - :param num_retries: - Number of upload retries. By default, only uploads with - if_generation_match set will be retried, as uploads without the - argument are not guaranteed to be idempotent. Setting num_retries - will override this default behavior and guarantee retries even when - if_generation_match is not set. (Deprecated: This argument - will be removed in a future release.) - :type client: :class:`~google.cloud.storage.client.Client` :param client: (Optional) The client to use. If not passed, falls back to the @@ -2662,14 +2604,6 @@ def _prep_and_do_upload( :raises: :class:`~google.cloud.exceptions.GoogleCloudError` if the upload response returns an error status. """ - if num_retries is not None: - warnings.warn(_NUM_RETRIES_MESSAGE, DeprecationWarning, stacklevel=2) - # num_retries and retry are mutually exclusive. If num_retries is - # set and retry is exactly the default, then nullify retry for - # backwards compatibility. - if retry is DEFAULT_RETRY_IF_GENERATION_SPECIFIED: - retry = None - _maybe_rewind(file_obj, rewind=rewind) predefined_acl = ACL.validate_predefined(predefined_acl) @@ -2679,7 +2613,6 @@ def _prep_and_do_upload( file_obj, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -2700,7 +2633,6 @@ def upload_from_file( rewind=False, size=None, content_type=None, - num_retries=None, client=None, predefined_acl=None, if_generation_match=None, @@ -2757,15 +2689,6 @@ def upload_from_file( :type content_type: str :param content_type: (Optional) Type of content being uploaded. - :type num_retries: int - :param num_retries: - Number of upload retries. By default, only uploads with - if_generation_match set will be retried, as uploads without the - argument are not guaranteed to be idempotent. Setting num_retries - will override this default behavior and guarantee retries even when - if_generation_match is not set. (Deprecated: This argument - will be removed in a future release.) - :type client: :class:`~google.cloud.storage.client.Client` :param client: (Optional) The client to use. If not passed, falls back to the @@ -2829,7 +2752,6 @@ def upload_from_file( rewind=rewind, size=size, content_type=content_type, - num_retries=num_retries, client=client, predefined_acl=predefined_acl, if_generation_match=if_generation_match, @@ -2869,7 +2791,6 @@ def upload_from_filename( self, filename, content_type=None, - num_retries=None, client=None, predefined_acl=None, if_generation_match=None, @@ -2918,15 +2839,6 @@ def upload_from_filename( (Optional) The client to use. If not passed, falls back to the ``client`` stored on the blob's bucket. - :type num_retries: int - :param num_retries: - Number of upload retries. By default, only uploads with - if_generation_match set will be retried, as uploads without the - argument are not guaranteed to be idempotent. Setting num_retries - will override this default behavior and guarantee retries even when - if_generation_match is not set. (Deprecated: This argument - will be removed in a future release.) - :type predefined_acl: str :param predefined_acl: (Optional) Predefined access control list @@ -2981,7 +2893,6 @@ def upload_from_filename( self._handle_filename_and_upload( filename, content_type=content_type, - num_retries=num_retries, client=client, predefined_acl=predefined_acl, if_generation_match=if_generation_match, @@ -2997,7 +2908,6 @@ def upload_from_string( self, data, content_type="text/plain", - num_retries=None, client=None, predefined_acl=None, if_generation_match=None, @@ -3033,15 +2943,6 @@ def upload_from_string( (Optional) Type of content being uploaded. Defaults to ``'text/plain'``. - :type num_retries: int - :param num_retries: - Number of upload retries. By default, only uploads with - if_generation_match set will be retried, as uploads without the - argument are not guaranteed to be idempotent. Setting num_retries - will override this default behavior and guarantee retries even when - if_generation_match is not set. (Deprecated: This argument - will be removed in a future release.) - :type client: :class:`~google.cloud.storage.client.Client` :param client: (Optional) The client to use. If not passed, falls back to the @@ -3103,7 +3004,6 @@ def upload_from_string( file_obj=string_buffer, size=len(data), content_type=content_type, - num_retries=num_retries, client=client, predefined_acl=predefined_acl, if_generation_match=if_generation_match, @@ -3271,7 +3171,6 @@ def create_resumable_upload_session( fake_stream, content_type, size, - None, predefined_acl=predefined_acl, if_generation_match=if_generation_match, if_generation_not_match=if_generation_not_match, @@ -4061,16 +3960,9 @@ def open( For uploads only, the following additional arguments are supported: - ``content_type`` - - ``num_retries`` - ``predefined_acl`` - ``checksum`` - .. note:: - - ``num_retries`` is supported for backwards-compatibility - reasons only; please use ``retry`` with a Retry object or - ConditionalRetryPolicy instead. - :type mode: str :param mode: (Optional) A mode string, as per standard Python `open()` semantics.The first diff --git a/google/cloud/storage/fileio.py b/google/cloud/storage/fileio.py index 985dd0012..c765811d1 100644 --- a/google/cloud/storage/fileio.py +++ b/google/cloud/storage/fileio.py @@ -15,10 +15,8 @@ """Module for file-like access of blobs, usually invoked via Blob.open().""" import io -import warnings from google.api_core.exceptions import RequestRangeNotSatisfiable -from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE from google.cloud.storage.retry import DEFAULT_RETRY from google.cloud.storage.retry import DEFAULT_RETRY_IF_GENERATION_SPECIFIED from google.cloud.storage.retry import ConditionalRetryPolicy @@ -45,7 +43,6 @@ VALID_UPLOAD_KWARGS = { "content_type", "predefined_acl", - "num_retries", "if_generation_match", "if_generation_not_match", "if_metageneration_match", @@ -291,7 +288,6 @@ class BlobWriter(io.BufferedIOBase): - ``if_metageneration_not_match`` - ``timeout`` - ``content_type`` - - ``num_retries`` - ``predefined_acl`` - ``checksum`` """ @@ -359,19 +355,9 @@ def write(self, b): return pos def _initiate_upload(self): - # num_retries is only supported for backwards-compatibility reasons. - num_retries = self._upload_kwargs.pop("num_retries", None) retry = self._retry content_type = self._upload_kwargs.pop("content_type", None) - if num_retries is not None: - warnings.warn(_NUM_RETRIES_MESSAGE, DeprecationWarning, stacklevel=2) - # num_retries and retry are mutually exclusive. If num_retries is - # set and retry is exactly the default, then nullify retry for - # backwards compatibility. - if retry is DEFAULT_RETRY_IF_GENERATION_SPECIFIED: - retry = None - # Handle ConditionalRetryPolicy. if isinstance(retry, ConditionalRetryPolicy): # Conditional retries are designed for non-media calls, which change @@ -391,7 +377,6 @@ def _initiate_upload(self): self._buffer, content_type, None, - num_retries, chunk_size=self._chunk_size, retry=retry, **self._upload_kwargs, diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index 401e0dd15..eb796adef 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -717,14 +717,6 @@ def test_hostname_and_scheme(self): class Test__api_core_retry_to_resumable_media_retry(unittest.TestCase): - def test_conflict(self): - from google.cloud.storage._helpers import ( - _api_core_retry_to_resumable_media_retry, - ) - - with self.assertRaises(ValueError): - _api_core_retry_to_resumable_media_retry(retry=DEFAULT_RETRY, num_retries=2) - def test_retry(self): from google.cloud.storage._helpers import ( _api_core_retry_to_resumable_media_retry, @@ -736,24 +728,6 @@ def test_retry(self): self.assertEqual(retry_strategy.initial_delay, DEFAULT_RETRY._initial) self.assertEqual(retry_strategy.multiplier, DEFAULT_RETRY._multiplier) - def test_num_retries(self): - from google.cloud.storage._helpers import ( - _api_core_retry_to_resumable_media_retry, - ) - - retry_strategy = _api_core_retry_to_resumable_media_retry( - retry=None, num_retries=2 - ) - self.assertEqual(retry_strategy.max_retries, 2) - - def test_none(self): - from google.cloud.storage._helpers import ( - _api_core_retry_to_resumable_media_retry, - ) - - retry_strategy = _api_core_retry_to_resumable_media_retry(retry=None) - self.assertEqual(retry_strategy.max_retries, 0) - class _MD5Hash(object): def __init__(self, digest_val): diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index a77252466..a96a223b3 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -2351,7 +2351,6 @@ def _do_multipart_success( mock_get_boundary, client=None, size=None, - num_retries=None, user_project=None, predefined_acl=None, if_generation_match=None, @@ -2407,7 +2406,6 @@ def _do_multipart_success( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -2505,26 +2503,6 @@ def test__do_multipart_upload_no_size_retry(self, mock_get_boundary): mock_get_boundary, predefined_acl="private", retry=DEFAULT_RETRY ) - @mock.patch( - "google.cloud.storage._media._upload.get_boundary", return_value=b"==0==" - ) - def test__do_multipart_upload_no_size_num_retries(self, mock_get_boundary): - self._do_multipart_success( - mock_get_boundary, predefined_acl="private", num_retries=2 - ) - - @mock.patch( - "google.cloud.storage._media._upload.get_boundary", return_value=b"==0==" - ) - def test__do_multipart_upload_no_size_retry_conflict(self, mock_get_boundary): - with self.assertRaises(ValueError): - self._do_multipart_success( - mock_get_boundary, - predefined_acl="private", - num_retries=2, - retry=DEFAULT_RETRY, - ) - @mock.patch( "google.cloud.storage._media._upload.get_boundary", return_value=b"==0==" ) @@ -2652,7 +2630,6 @@ def _initiate_resumable_helper( size=None, extra_headers=None, chunk_size=None, - num_retries=None, user_project=None, predefined_acl=None, if_generation_match=None, @@ -2727,7 +2704,6 @@ def _initiate_resumable_helper( stream, content_type, size, - num_retries, extra_headers=extra_headers, chunk_size=chunk_size, predefined_acl=predefined_acl, @@ -2813,10 +2789,7 @@ def _initiate_resumable_helper( self.assertEqual(upload._content_type, content_type) self.assertEqual(upload.resumable_url, resumable_url) retry_strategy = upload._retry_strategy - self.assertFalse(num_retries is not None and retry is not None) - if num_retries is not None and retry is None: - self.assertEqual(retry_strategy.max_retries, num_retries) - elif retry is None: + if retry is None: self.assertEqual(retry_strategy.max_retries, 0) else: self.assertEqual(retry_strategy.max_sleep, 60.0) @@ -2906,13 +2879,6 @@ def test__initiate_resumable_upload_with_extra_headers(self): def test__initiate_resumable_upload_with_retry(self): self._initiate_resumable_helper(retry=DEFAULT_RETRY) - def test__initiate_resumable_upload_w_num_retries(self): - self._initiate_resumable_helper(num_retries=11) - - def test__initiate_resumable_upload_with_retry_conflict(self): - with self.assertRaises(ValueError): - self._initiate_resumable_helper(retry=DEFAULT_RETRY, num_retries=2) - def test__initiate_resumable_upload_with_generation_match(self): self._initiate_resumable_helper( if_generation_match=4, if_metageneration_match=4 @@ -3075,7 +3041,6 @@ def _do_resumable_upload_call2( def _do_resumable_helper( self, use_size=False, - num_retries=None, predefined_acl=None, if_generation_match=None, if_generation_not_match=None, @@ -3145,7 +3110,6 @@ def _do_resumable_helper( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -3214,13 +3178,6 @@ def test__do_resumable_upload_with_size(self): def test__do_resumable_upload_with_retry(self): self._do_resumable_helper(retry=DEFAULT_RETRY) - def test__do_resumable_upload_w_num_retries(self): - self._do_resumable_helper(num_retries=8) - - def test__do_resumable_upload_with_retry_conflict(self): - with self.assertRaises(ValueError): - self._do_resumable_helper(num_retries=9, retry=DEFAULT_RETRY) - def test__do_resumable_upload_with_predefined_acl(self): self._do_resumable_helper(predefined_acl="private") @@ -3235,7 +3192,6 @@ def test__do_resumable_upload_with_data_corruption(self): def _do_upload_helper( self, chunk_size=None, - num_retries=None, predefined_acl=None, if_generation_match=None, if_generation_not_match=None, @@ -3281,7 +3237,6 @@ def _do_upload_helper( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -3302,7 +3257,6 @@ def _do_upload_helper( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -3321,7 +3275,6 @@ def _do_upload_helper( stream, content_type, size, - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -3360,9 +3313,6 @@ def test__do_upload_uses_resumable_w_custom_timeout(self): def test__do_upload_with_retry(self): self._do_upload_helper(retry=DEFAULT_RETRY) - def test__do_upload_w_num_retries(self): - self._do_upload_helper(num_retries=2) - def test__do_upload_with_conditional_retry_success(self): self._do_upload_helper( retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, if_generation_match=123456 @@ -3391,10 +3341,7 @@ def _upload_from_file_helper(self, side_effect=None, **kwargs): if_generation_not_match = kwargs.get("if_generation_not_match", None) if_metageneration_match = kwargs.get("if_metageneration_match", None) if_metageneration_not_match = kwargs.get("if_metageneration_not_match", None) - num_retries = kwargs.get("num_retries", None) - default_retry = ( - DEFAULT_RETRY_IF_GENERATION_SPECIFIED if not num_retries else None - ) + default_retry = DEFAULT_RETRY_IF_GENERATION_SPECIFIED retry = kwargs.get("retry", default_retry) ret_val = blob.upload_from_file( stream, size=len(data), content_type=content_type, client=client, **kwargs @@ -3412,7 +3359,6 @@ def _upload_from_file_helper(self, side_effect=None, **kwargs): stream, content_type, len(data), - num_retries, predefined_acl, if_generation_match, if_generation_not_match, @@ -3432,33 +3378,6 @@ def test_upload_from_file_success(self): def test_upload_from_file_with_retry(self): self._upload_from_file_helper(retry=DEFAULT_RETRY) - @mock.patch("warnings.warn") - def test_upload_from_file_w_num_retries(self, mock_warn): - from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE - - self._upload_from_file_helper(num_retries=2) - - mock_warn.assert_called_once_with( - _NUM_RETRIES_MESSAGE, - DeprecationWarning, - stacklevel=2, - ) - - @mock.patch("warnings.warn") - def test_upload_from_file_with_retry_conflict(self, mock_warn): - from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE - - # Special case here: in a conflict this method should NOT raise an error - # as that's handled further downstream. It should pass both options - # through. - self._upload_from_file_helper(retry=DEFAULT_RETRY, num_retries=2) - - mock_warn.assert_called_once_with( - _NUM_RETRIES_MESSAGE, - DeprecationWarning, - stacklevel=2, - ) - def test_upload_from_file_with_rewind(self): stream = self._upload_from_file_helper(rewind=True) assert stream.tell() == 0 @@ -3490,27 +3409,25 @@ def _do_upload_mock_call_helper( content_type, size, timeout=None, - num_retries=None, retry=None, ): self.assertEqual(blob._do_upload.call_count, 1) mock_call = blob._do_upload.mock_calls[0] call_name, pos_args, kwargs = mock_call self.assertEqual(call_name, "") - self.assertEqual(len(pos_args), 10) + self.assertEqual(len(pos_args), 9) self.assertEqual(pos_args[0], client) self.assertEqual(pos_args[2], content_type) self.assertEqual(pos_args[3], size) - self.assertEqual(pos_args[4], num_retries) # num_retries - self.assertIsNone(pos_args[5]) # predefined_acl - self.assertIsNone(pos_args[6]) # if_generation_match - self.assertIsNone(pos_args[7]) # if_generation_not_match - self.assertIsNone(pos_args[8]) # if_metageneration_match - self.assertIsNone(pos_args[9]) # if_metageneration_not_match + self.assertIsNone(pos_args[4]) # predefined_acl + self.assertIsNone(pos_args[5]) # if_generation_match + self.assertIsNone(pos_args[6]) # if_generation_not_match + self.assertIsNone(pos_args[7]) # if_metageneration_match + self.assertIsNone(pos_args[8]) # if_metageneration_not_match expected_timeout = self._get_default_timeout() if timeout is None else timeout if not retry: - retry = DEFAULT_RETRY_IF_GENERATION_SPECIFIED if not num_retries else None + retry = DEFAULT_RETRY_IF_GENERATION_SPECIFIED self.assertEqual( kwargs, { @@ -3587,47 +3504,6 @@ def test_upload_from_filename_with_retry(self): self.assertEqual(stream.mode, "rb") self.assertEqual(stream.name, temp.name) - @mock.patch("warnings.warn") - def test_upload_from_filename_w_num_retries(self, mock_warn): - from google.cloud._testing import _NamedTemporaryFile - from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE - - blob = self._make_one("blob-name", bucket=None) - # Mock low-level upload helper on blob (it is tested elsewhere). - created_json = {"metadata": {"mint": "ice-cream"}} - blob._do_upload = mock.Mock(return_value=created_json, spec=[]) - # Make sure `metadata` is empty before the request. - self.assertIsNone(blob.metadata) - - data = b"soooo much data" - content_type = "image/svg+xml" - client = mock.sentinel.client - with _NamedTemporaryFile() as temp: - with open(temp.name, "wb") as file_obj: - file_obj.write(data) - - ret_val = blob.upload_from_filename( - temp.name, content_type=content_type, client=client, num_retries=2 - ) - - # Check the response and side-effects. - self.assertIsNone(ret_val) - self.assertEqual(blob.metadata, created_json["metadata"]) - - # Check the mock. - stream = self._do_upload_mock_call_helper( - blob, client, content_type, len(data), num_retries=2 - ) - self.assertTrue(stream.closed) - self.assertEqual(stream.mode, "rb") - self.assertEqual(stream.name, temp.name) - - mock_warn.assert_called_once_with( - _NUM_RETRIES_MESSAGE, - DeprecationWarning, - stacklevel=2, - ) - def test_upload_from_filename_w_custom_timeout(self): from google.cloud._testing import _NamedTemporaryFile @@ -3675,8 +3551,6 @@ def _upload_from_string_helper(self, data, **kwargs): extra_kwargs = {} if "retry" in kwargs: extra_kwargs["retry"] = kwargs["retry"] - if "num_retries" in kwargs: - extra_kwargs["num_retries"] = kwargs["num_retries"] # Check the mock. payload = _to_bytes(data, encoding="utf-8") stream = self._do_upload_mock_call_helper( @@ -3706,19 +3580,6 @@ def test_upload_from_string_w_text_w_retry(self): data = "\N{snowman} \N{sailboat}" self._upload_from_string_helper(data, retry=DEFAULT_RETRY) - @mock.patch("warnings.warn") - def test_upload_from_string_with_num_retries(self, mock_warn): - from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE - - data = "\N{snowman} \N{sailboat}" - self._upload_from_string_helper(data, num_retries=2) - - mock_warn.assert_called_once_with( - _NUM_RETRIES_MESSAGE, - DeprecationWarning, - stacklevel=2, - ) - def _create_resumable_upload_session_helper( self, origin=None, diff --git a/tests/unit/test_fileio.py b/tests/unit/test_fileio.py index b2cb4b2a9..aae84d73e 100644 --- a/tests/unit/test_fileio.py +++ b/tests/unit/test_fileio.py @@ -28,7 +28,6 @@ TEST_BINARY_DATA = TEST_TEXT_DATA.encode("utf-8") TEST_MULTIBYTE_TEXT_DATA = "あいうえおかきくけこさしすせそたちつてと" PLAIN_CONTENT_TYPE = "text/plain" -NUM_RETRIES = 2 class _BlobReaderBase: @@ -342,8 +341,6 @@ def test_reject_wrong_chunk_size(self): @mock.patch("warnings.warn") def test_write(self, mock_warn): - from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE - blob = mock.Mock() upload = mock.Mock() transport = mock.Mock() @@ -364,7 +361,6 @@ def test_write(self, mock_warn): writer = self._make_blob_writer( blob, chunk_size=chunk_size, - num_retries=NUM_RETRIES, content_type=PLAIN_CONTENT_TYPE, **upload_kwargs ) @@ -388,7 +384,6 @@ def test_write(self, mock_warn): writer._buffer, PLAIN_CONTENT_TYPE, None, - NUM_RETRIES, chunk_size=chunk_size, retry=None, **upload_kwargs @@ -402,12 +397,6 @@ def test_write(self, mock_warn): writer.close() self.assertEqual(upload.transmit_next_chunk.call_count, 5) - mock_warn.assert_called_once_with( - _NUM_RETRIES_MESSAGE, - DeprecationWarning, - stacklevel=2, - ) - def test_close_errors(self): blob = mock.Mock(chunk_size=None) @@ -526,7 +515,6 @@ def test_conditional_retry_failure(self): writer._buffer, PLAIN_CONTENT_TYPE, None, # size - None, # num_retries chunk_size=chunk_size, retry=None, ) @@ -578,7 +566,6 @@ def test_conditional_retry_pass(self): writer._buffer, PLAIN_CONTENT_TYPE, None, # size - None, # num_retries chunk_size=chunk_size, retry=DEFAULT_RETRY, if_generation_match=123456, @@ -630,117 +617,12 @@ def test_forced_default_retry(self): writer._buffer, PLAIN_CONTENT_TYPE, None, # size - None, # num_retries - chunk_size=chunk_size, - retry=DEFAULT_RETRY, - ) - upload.transmit_next_chunk.assert_called_with(transport) - self.assertEqual(upload.transmit_next_chunk.call_count, 4) - - # Write another byte, finalize and close. - writer.write(TEST_BINARY_DATA[32:33]) - writer.close() - self.assertEqual(upload.transmit_next_chunk.call_count, 5) - - @mock.patch("warnings.warn") - def test_num_retries_and_retry_conflict(self, mock_warn): - from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE - - blob = mock.Mock() - - blob._initiate_resumable_upload.side_effect = ValueError - - with mock.patch("google.cloud.storage.fileio.CHUNK_SIZE_MULTIPLE", 1): - # Create a writer. - # It would be normal to use a context manager here, but not doing so - # gives us more control over close() for test purposes. - chunk_size = 8 # Note: Real upload requires a multiple of 256KiB. - writer = self._make_blob_writer( - blob, - chunk_size=chunk_size, - content_type=PLAIN_CONTENT_TYPE, - num_retries=2, - retry=DEFAULT_RETRY, - ) - - # Write under chunk_size. This should be buffered and the upload not - # initiated. - writer.write(TEST_BINARY_DATA[0:4]) - blob._initiate_resumable_upload.assert_not_called() - - # Write over chunk_size. The mock will raise a ValueError, simulating - # actual behavior when num_retries and retry are both specified. - with self.assertRaises(ValueError): - writer.write(TEST_BINARY_DATA[4:32]) - - blob._initiate_resumable_upload.assert_called_once_with( - blob.bucket.client, - writer._buffer, - PLAIN_CONTENT_TYPE, - None, # size - 2, # num_retries chunk_size=chunk_size, retry=DEFAULT_RETRY, ) - - mock_warn.assert_called_once_with( - _NUM_RETRIES_MESSAGE, - DeprecationWarning, - stacklevel=2, - ) - - @mock.patch("warnings.warn") - def test_num_retries_only(self, mock_warn): - from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE - - blob = mock.Mock() - upload = mock.Mock() - transport = mock.Mock() - - blob._initiate_resumable_upload.return_value = (upload, transport) - - with mock.patch("google.cloud.storage.fileio.CHUNK_SIZE_MULTIPLE", 1): - # Create a writer. - # It would be normal to use a context manager here, but not doing so - # gives us more control over close() for test purposes. - chunk_size = 8 # Note: Real upload requires a multiple of 256KiB. - writer = self._make_blob_writer( - blob, - chunk_size=chunk_size, - content_type=PLAIN_CONTENT_TYPE, - num_retries=2, - ) - - # The transmit_next_chunk method must actually consume bytes from the - # sliding buffer for the flush() feature to work properly. - upload.transmit_next_chunk.side_effect = lambda _: writer._buffer.read( - chunk_size - ) - - # Write under chunk_size. This should be buffered and the upload not - # initiated. - writer.write(TEST_BINARY_DATA[0:4]) - blob._initiate_resumable_upload.assert_not_called() - - # Write over chunk_size. This should result in upload initialization - # and multiple chunks uploaded. - writer.write(TEST_BINARY_DATA[4:32]) - blob._initiate_resumable_upload.assert_called_once_with( - blob.bucket.client, - writer._buffer, - PLAIN_CONTENT_TYPE, - None, # size - 2, # num_retries - chunk_size=chunk_size, - retry=None, - ) upload.transmit_next_chunk.assert_called_with(transport) self.assertEqual(upload.transmit_next_chunk.call_count, 4) - mock_warn.assert_called_once_with( - _NUM_RETRIES_MESSAGE, DeprecationWarning, stacklevel=2 - ) - # Write another byte, finalize and close. writer.write(TEST_BINARY_DATA[32:33]) writer.close() @@ -980,8 +862,6 @@ def test_close(self): class TestBlobWriterText(unittest.TestCase, _BlobWriterBase): @mock.patch("warnings.warn") def test_write(self, mock_warn): - from google.cloud.storage._helpers import _NUM_RETRIES_MESSAGE - blob = mock.Mock() upload = mock.Mock() transport = mock.Mock() @@ -997,7 +877,6 @@ def test_write(self, mock_warn): blob, chunk_size=chunk_size, ignore_flush=True, - num_retries=NUM_RETRIES, content_type=PLAIN_CONTENT_TYPE, ) @@ -1023,14 +902,7 @@ def test_write(self, mock_warn): unwrapped_writer._buffer, PLAIN_CONTENT_TYPE, None, - NUM_RETRIES, chunk_size=chunk_size, retry=None, ) upload.transmit_next_chunk.assert_called_with(transport) - - mock_warn.assert_called_once_with( - _NUM_RETRIES_MESSAGE, - DeprecationWarning, - stacklevel=2, - )