From 7b947ec2b241ed970058e2aecad7dc3bc527afd4 Mon Sep 17 00:00:00 2001 From: ArzelaAscoIi <37148029+ArzelaAscoIi@users.noreply.github.com> Date: Mon, 4 Sep 2023 14:33:58 +0200 Subject: [PATCH] feat: error message for failed upload (#99) * feat: error message for failed upload * fix: test returns exceptions * fix * fix: format * chore: bump version --- deepset_cloud_sdk/__about__.py | 2 +- deepset_cloud_sdk/_s3/upload.py | 31 ++++++++++++++++--------------- tests/unit/s3/test_upload.py | 10 +++++++--- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/deepset_cloud_sdk/__about__.py b/deepset_cloud_sdk/__about__.py index 0b48645b..7a8c3211 100644 --- a/deepset_cloud_sdk/__about__.py +++ b/deepset_cloud_sdk/__about__.py @@ -1,3 +1,3 @@ """This file defines the package version.""" -__version__ = "0.0.28" +__version__ = "0.0.29" diff --git a/deepset_cloud_sdk/_s3/upload.py b/deepset_cloud_sdk/_s3/upload.py index 9d1d8209..ef54e678 100644 --- a/deepset_cloud_sdk/_s3/upload.py +++ b/deepset_cloud_sdk/_s3/upload.py @@ -32,16 +32,6 @@ def __init__(self, error: aiohttp.ClientResponseError) -> None: self.error = error -@dataclass -class S3UploadSummary: - """A summary of the S3 upload results.""" - - total_files: int - successful_upload_count: int - failed_upload_count: int - failed: List[str] - - @dataclass class S3UploadResult: """Stores the result of an upload to S3.""" @@ -51,6 +41,16 @@ class S3UploadResult: exception: Optional[Exception] = None +@dataclass +class S3UploadSummary: + """A summary of the S3 upload results.""" + + total_files: int + successful_upload_count: int + failed_upload_count: int + failed: List[S3UploadResult] + + def make_safe_file_name(file_name: str) -> str: """ Transform a given string to a representation that S3 accepts. @@ -164,13 +164,14 @@ async def upload_from_file( try: await self._upload_file_with_retries(file_name, upload_session, content, client_session) return S3UploadResult(file_name=file_name, success=True) - except: # pylint: disable=bare-except - logger.warn( + except Exception as exception: # pylint: disable=broad-exception-caught + logger.error( "Could not upload a file to deepset Cloud", file_name=file_name, session_id=upload_session.session_id, + exception=str(exception), ) - return S3UploadResult(file_name=file_name, success=False) + return S3UploadResult(file_name=file_name, success=False, exception=exception) async def upload_from_string( self, @@ -220,13 +221,13 @@ async def _process_results( failed_files=[r for r in results if not r.success], ) - failed: List[str] = [] + failed: List[S3UploadResult] = [] successfully_uploaded = 0 for result in results: if result.success: successfully_uploaded += 1 else: - failed.append(result.file_name) + failed.append(result) result_summary = S3UploadSummary( successful_upload_count=successfully_uploaded, diff --git a/tests/unit/s3/test_upload.py b/tests/unit/s3/test_upload.py index 57d92004..94639088 100644 --- a/tests/unit/s3/test_upload.py +++ b/tests/unit/s3/test_upload.py @@ -113,7 +113,7 @@ async def test_upload_files_from_path_http_error(self, upload_session_response: assert results.successful_upload_count == 0 assert results.failed_upload_count == 6 assert len(results.failed) == 6 - assert results.failed == [ + assert [f.file_name for f in results.failed] == [ "16675.txt", "16675.txt.meta.json", "22297.txt", @@ -121,6 +121,7 @@ async def test_upload_files_from_path_http_error(self, upload_session_response: "35887.txt", "35887.txt.meta.json", ] + assert all(isinstance(f.exception, RetryableHttpError) for f in results.failed) async def test_upload_texts_http_error(self, upload_session_response: UploadSession) -> None: exception = aiohttp.ClientResponseError(request_info=Mock(), history=Mock(), status=503) @@ -138,11 +139,13 @@ async def test_upload_texts_http_error(self, upload_session_response: UploadSess assert results.successful_upload_count == 0 assert results.failed_upload_count == 3 assert len(results.failed) == 3 - assert results.failed == [ + + assert [f.file_name for f in results.failed] == [ "one.txt", "two.txt", "three.txt", ] + assert all(isinstance(f.exception, RetryableHttpError) for f in results.failed) async def test_upload_texts_with_metadata_http_error(self, upload_session_response: UploadSession) -> None: exception = aiohttp.ClientResponseError(request_info=Mock(), history=Mock(), status=503) @@ -160,7 +163,7 @@ async def test_upload_texts_with_metadata_http_error(self, upload_session_respon assert results.successful_upload_count == 0 assert results.failed_upload_count == 6 assert len(results.failed) == 6 - assert results.failed == [ + assert [f.file_name for f in results.failed] == [ "one.txt", "one.txt.meta.json", "two.txt", @@ -168,6 +171,7 @@ async def test_upload_texts_with_metadata_http_error(self, upload_session_respon "three.txt", "three.txt.meta.json", ] + assert all(isinstance(f.exception, RetryableHttpError) for f in results.failed) @pytest.mark.parametrize("status", [503, 502, 500, 504, 408]) @patch("aiohttp.ClientSession")