From 7f10a3ca5c2752733fb636463a929f8114388034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=C5=81azowik?= Date: Thu, 10 Mar 2022 16:57:58 +0100 Subject: [PATCH 1/3] [gcloud] Add a setting for connect/read timeouts --- docs/backends/gcloud.rst | 16 ++++++++++++++++ storages/backends/gcloud.py | 24 +++++++++++++++--------- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/docs/backends/gcloud.rst b/docs/backends/gcloud.rst index 9a87ef36a..e1e22e8a2 100644 --- a/docs/backends/gcloud.rst +++ b/docs/backends/gcloud.rst @@ -204,6 +204,22 @@ Note: Default Google Compute Engine (GCE) Service accounts are The ``GS_EXPIRATION`` value is handled by the underlying `Google library `_. It supports `timedelta`, `datetime`, or `integer` seconds since epoch time. +``GS_TIMEOUT`` (optional: default is ``60``, float or tuple) + +Connect/read timeout. The amount of time, in seconds, to wait for the connection to the server to establish, and between +bytes sent from the server. If float is given it's applied to both, if a tuple – the first value is for the connect +timeout, second for read. + +Note that read timeout =/= download timeout. It’s the number of seconds that the client will wait *between* bytes sent +from the server. In 99.9% of cases, this is the time before the server sends the first byte. + +See https://docs.python-requests.org/en/master/user/advanced/#timeouts + +Sometimes requests can get stuck, so it's better if the timeout is low, couple of seconds. This means that a new request +(via retry) will be made sooner. The default is higher to keep the behavior from before this setting was introduced. + +Timeouts will be automatically retried when using `google-cloud-storage` version that includes +https://github.com/googleapis/python-storage/pull/727 Usage ----- diff --git a/storages/backends/gcloud.py b/storages/backends/gcloud.py index 15aa2e5d4..5798eb300 100644 --- a/storages/backends/gcloud.py +++ b/storages/backends/gcloud.py @@ -34,7 +34,7 @@ def __init__(self, name, mode, storage): self.mime_type = mimetypes.guess_type(name)[0] self._mode = mode self._storage = storage - self.blob = storage.bucket.get_blob(name) + self.blob = storage.bucket.get_blob(name, timeout=storage.timeout) if not self.blob and 'w' in mode: self.blob = Blob( self.name, storage.bucket, @@ -55,7 +55,7 @@ def _get_file(self): ) if 'r' in self._mode: self._is_dirty = False - self.blob.download_to_file(self._file) + self.blob.download_to_file(self._file, timeout=self._storage.timeout) self._file.seek(0) if self._storage.gzip and self.blob.content_encoding == 'gzip': self._file = self._decompress_file(mode=self._mode, file=self._file) @@ -87,7 +87,8 @@ def close(self): blob_params = self._storage.get_object_parameters(self.name) self.blob.upload_from_file( self.file, rewind=True, content_type=self.mime_type, - predefined_acl=blob_params.get('acl', self._storage.default_acl)) + predefined_acl=blob_params.get('acl', self._storage.default_acl), + timeout=self._storage.timeout) self._file.close() self._file = None @@ -128,6 +129,7 @@ def get_default_settings(self): # roll over. "max_memory_size": setting('GS_MAX_MEMORY_SIZE', 0), "blob_chunk_size": setting('GS_BLOB_CHUNK_SIZE'), + "timeout": setting('GS_TIMEOUT', 60) } @property @@ -186,7 +188,10 @@ def _save(self, name, content): for prop, val in blob_params.items(): setattr(file_object.blob, prop, val) - file_object.blob.upload_from_file(content, rewind=True, size=getattr(content, 'size', None), **upload_params) + file_object.blob.upload_from_file(content, rewind=True, + size=getattr(content, 'size', None), + timeout=self.timeout, + **upload_params) return cleaned_name def get_object_parameters(self, name): @@ -209,20 +214,20 @@ def get_object_parameters(self, name): def delete(self, name): name = self._normalize_name(clean_name(name)) try: - self.bucket.delete_blob(name) + self.bucket.delete_blob(name, timeout=self.timeout) except NotFound: pass def exists(self, name): if not name: # root element aka the bucket try: - self.client.get_bucket(self.bucket) + self.client.get_bucket(self.bucket, timeout=self.timeout) return True except NotFound: return False name = self._normalize_name(clean_name(name)) - return bool(self.bucket.get_blob(name)) + return bool(self.bucket.get_blob(name, timeout=self.timeout)) def listdir(self, name): name = self._normalize_name(clean_name(name)) @@ -231,7 +236,8 @@ def listdir(self, name): if name and not name.endswith('/'): name += '/' - iterator = self.bucket.list_blobs(prefix=name, delimiter='/') + iterator = self.bucket.list_blobs(prefix=name, delimiter='/', + timeout=self.timeout) blobs = list(iterator) prefixes = iterator.prefixes @@ -249,7 +255,7 @@ def listdir(self, name): def _get_blob(self, name): # Wrap google.cloud.storage's blob to raise if the file doesn't exist - blob = self.bucket.get_blob(name) + blob = self.bucket.get_blob(name, timeout=self.timeout) if blob is None: raise NotFound('File does not exist: {}'.format(name)) From 57a592e1c32bd07c031fa1391bb3d0d363c9dde5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=C5=81azowik?= Date: Thu, 10 Mar 2022 17:19:14 +0100 Subject: [PATCH 2/3] [tests/gcloud] Add timeout to expected params --- tests/test_gcloud.py | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/tests/test_gcloud.py b/tests/test_gcloud.py index 17d912c8d..758c66e4e 100644 --- a/tests/test_gcloud.py +++ b/tests/test_gcloud.py @@ -38,7 +38,7 @@ def test_open_read(self): f = self.storage.open(self.filename) self.storage._client.bucket.assert_called_with(self.bucket_name) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) f.blob.download_to_file = lambda tmpfile: tmpfile.write(data) self.assertEqual(f.read(), data) @@ -49,7 +49,7 @@ def test_open_read_num_bytes(self): f = self.storage.open(self.filename) self.storage._client.bucket.assert_called_with(self.bucket_name) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) f.blob.download_to_file = lambda tmpfile: tmpfile.write(data) self.assertEqual(f.read(num_bytes), data[0:num_bytes]) @@ -59,7 +59,7 @@ def test_open_read_nonexistent(self): self.storage._bucket.get_blob.return_value = None self.assertRaises(FileNotFoundError, self.storage.open, self.filename) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) def test_open_read_nonexistent_unicode(self): filename = 'ủⓝï℅ⅆℇ.txt' @@ -92,7 +92,7 @@ def test_open_write(self, MockBlob): MockBlob().upload_from_file.assert_called_with( tmpfile, rewind=True, content_type=mimetypes.guess_type(self.filename)[0], - predefined_acl='projectPrivate') + predefined_acl='projectPrivate', timeout=60) def test_save(self): data = 'This is some test content.' @@ -103,7 +103,7 @@ def test_save(self): self.storage._client.bucket.assert_called_with(self.bucket_name) self.storage._bucket.get_blob().upload_from_file.assert_called_with( content, rewind=True, size=len(data), content_type=mimetypes.guess_type(self.filename)[0], - predefined_acl=None) + predefined_acl=None, timeout=60) def test_save2(self): data = 'This is some test ủⓝï℅ⅆℇ content.' @@ -115,7 +115,7 @@ def test_save2(self): self.storage._client.bucket.assert_called_with(self.bucket_name) self.storage._bucket.get_blob().upload_from_file.assert_called_with( content, rewind=True, size=len(data), content_type=mimetypes.guess_type(filename)[0], - predefined_acl=None) + predefined_acl=None, timeout=60) def test_save_with_default_acl(self): data = 'This is some test ủⓝï℅ⅆℇ content.' @@ -132,23 +132,23 @@ def test_save_with_default_acl(self): self.storage._client.bucket.assert_called_with(self.bucket_name) self.storage._bucket.get_blob().upload_from_file.assert_called_with( content, rewind=True, size=len(data), content_type=mimetypes.guess_type(filename)[0], - predefined_acl='publicRead') + predefined_acl='publicRead', timeout=60) def test_delete(self): self.storage.delete(self.filename) self.storage._client.bucket.assert_called_with(self.bucket_name) - self.storage._bucket.delete_blob.assert_called_with(self.filename) + self.storage._bucket.delete_blob.assert_called_with(self.filename, timeout=60) def test_exists(self): self.storage._bucket = mock.MagicMock() self.assertTrue(self.storage.exists(self.filename)) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) self.storage._bucket.reset_mock() self.storage._bucket.get_blob.return_value = None self.assertFalse(self.storage.exists(self.filename)) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) def test_exists_no_bucket(self): # exists('') should return False if the bucket doesn't exist @@ -233,7 +233,7 @@ def test_size(self): self.storage._bucket.get_blob.return_value = blob self.assertEqual(self.storage.size(self.filename), size) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) def test_size_no_file(self): self.storage._bucket = mock.MagicMock() @@ -254,7 +254,7 @@ def test_modified_time(self): mt = self.storage.modified_time(self.filename) self.assertTrue(timezone.is_naive(mt)) self.assertEqual(mt, naive_date) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) def test_get_modified_time(self): naive_date = datetime(2017, 1, 2, 3, 4, 5, 678) @@ -270,13 +270,13 @@ def test_get_modified_time(self): self.assertTrue(timezone.is_naive(mt)) naive_date_montreal = timezone.make_naive(aware_date) self.assertEqual(mt, naive_date_montreal) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) with self.settings(TIME_ZONE='America/Montreal', USE_TZ=True): mt = self.storage.get_modified_time(self.filename) self.assertTrue(timezone.is_aware(mt)) self.assertEqual(mt, aware_date) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) def test_get_created_time(self): naive_date = datetime(2017, 1, 2, 3, 4, 5, 678) @@ -292,13 +292,13 @@ def test_get_created_time(self): self.assertTrue(timezone.is_naive(mt)) naive_date_montreal = timezone.make_naive(aware_date) self.assertEqual(mt, naive_date_montreal) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) with self.settings(TIME_ZONE='America/Montreal', USE_TZ=True): mt = self.storage.get_created_time(self.filename) self.assertTrue(timezone.is_aware(mt)) self.assertEqual(mt, aware_date) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) def test_modified_time_no_file(self): self.storage._bucket = mock.MagicMock() @@ -385,7 +385,7 @@ def test_get_available_name(self): self.storage.file_overwrite = False self.assertEqual(self.storage.get_available_name( self.filename), self.filename) - self.storage._bucket.get_blob.assert_called_with(self.filename) + self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) def test_get_available_name_unicode(self): filename = 'ủⓝï℅ⅆℇ.txt' @@ -417,7 +417,8 @@ def test_storage_save_gzipped(self): rewind=True, size=11, predefined_acl=None, - content_type=None + content_type=None, + timeout=60, ) def test_storage_save_gzipped_non_seekable(self): @@ -433,7 +434,8 @@ def test_storage_save_gzipped_non_seekable(self): rewind=True, size=11, predefined_acl=None, - content_type=None + content_type=None, + timeout=60, ) def test_storage_save_gzip(self): @@ -453,6 +455,7 @@ def test_storage_save_gzip(self): size=None, predefined_acl=None, content_type='text/css', + timeout=60, ) args, kwargs = obj.upload_from_file.call_args content = args[0] @@ -482,6 +485,7 @@ def test_storage_save_gzip_twice(self): size=None, predefined_acl=None, content_type='text/css', + timeout=60, ) args, kwargs = obj.upload_from_file.call_args content = args[0] From 4ee9405a9cd397ec07519eb65c89446c018bf9f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20=C5=81azowik?= Date: Thu, 10 Mar 2022 17:40:00 +0100 Subject: [PATCH 3/3] [tests/gcloud] Fix mocks --- tests/test_gcloud.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_gcloud.py b/tests/test_gcloud.py index 758c66e4e..eec346097 100644 --- a/tests/test_gcloud.py +++ b/tests/test_gcloud.py @@ -40,7 +40,7 @@ def test_open_read(self): self.storage._client.bucket.assert_called_with(self.bucket_name) self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) - f.blob.download_to_file = lambda tmpfile: tmpfile.write(data) + f.blob.download_to_file = lambda tmpfile, **kwargs: tmpfile.write(data) self.assertEqual(f.read(), data) def test_open_read_num_bytes(self): @@ -51,7 +51,7 @@ def test_open_read_num_bytes(self): self.storage._client.bucket.assert_called_with(self.bucket_name) self.storage._bucket.get_blob.assert_called_with(self.filename, timeout=60) - f.blob.download_to_file = lambda tmpfile: tmpfile.write(data) + f.blob.download_to_file = lambda tmpfile, **kwargs: tmpfile.write(data) self.assertEqual(f.read(num_bytes), data[0:num_bytes]) def test_open_read_nonexistent(self):