From 82a835363953538e506f91eb3199d835f0624975 Mon Sep 17 00:00:00 2001 From: Alex Schofield Date: Thu, 22 Aug 2024 12:03:38 +0100 Subject: fix: change default parameters for bucket_name and client --- src/extract_lambda.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/extract_lambda.py b/src/extract_lambda.py index 24f0981..0e6dd8c 100644 --- a/src/extract_lambda.py +++ b/src/extract_lambda.py @@ -99,7 +99,9 @@ def connect_to_database() -> Connection: raise DBConnectionException("Failed to connect to database") -def extract_bucket(client=boto3.client("s3")): +def extract_bucket(client=None): + if client is None: + client = boto3.client("s3") response = client.list_buckets() extract_bucket_filter = [ bucket["Name"] for bucket in response["Buckets"] if "extract" in bucket["Name"] @@ -108,11 +110,16 @@ def extract_bucket(client=boto3.client("s3")): return extract_bucket_filter[0] -def list_existing_s3_files(bucket_name=extract_bucket(), client=boto3.client("s3")): +def list_existing_s3_files(bucket_name=None, client=None): """Creates a dictionary and populates it with the results of listing the contents of the s3 bucket, then returns the populated dictionary """ + if client is None: + client = boto3.client("s3") + if bucket_name is None: + bucket_name = extract_bucket(client) + logging.info("Listing existing S3 files") existing_files = {} -- cgit v1.2.3 From dc7dfe29ce977f3038fb3affd617683e8f163dc8 Mon Sep 17 00:00:00 2001 From: Alex Schofield Date: Thu, 22 Aug 2024 12:27:55 +0100 Subject: fix: handle no buckets properly --- src/extract_lambda.py | 3 +++ tests/test_extract_lambda.py | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/extract_lambda.py b/src/extract_lambda.py index 0e6dd8c..874098b 100644 --- a/src/extract_lambda.py +++ b/src/extract_lambda.py @@ -107,6 +107,9 @@ def extract_bucket(client=None): bucket["Name"] for bucket in response["Buckets"] if "extract" in bucket["Name"] ] + if not extract_bucket_filter: + raise ValueError("No extract_bucket found") + return extract_bucket_filter[0] diff --git a/tests/test_extract_lambda.py b/tests/test_extract_lambda.py index 1266cbb..bba433c 100644 --- a/tests/test_extract_lambda.py +++ b/tests/test_extract_lambda.py @@ -184,10 +184,8 @@ class TestExtractBucket: result = extract_bucket(s3_client) assert result == "extract_bucket" - def test_returns_index_error_if_no_buckets(self, s3_client): - # We don't even need to delete the bucket as there are no buckets - # due to the mock being reset for each test function now - with pytest.raises(IndexError, match="list index out of range"): + def test_raises_value_error_if_no_buckets(self, s3_client): + with pytest.raises(ValueError, match="No extract_bucket found"): extract_bucket(s3_client) @@ -196,7 +194,9 @@ class TestListExistingS3Files: logger = logging.getLogger() logger.info("Testing now.") caplog.set_level(logging.ERROR) - list_existing_s3_files(client=s3_client) + + with pytest.raises(ValueError, match="No extract_bucket found"): + list_existing_s3_files(client=s3_client) assert "Error listing S3 objects" in caplog.text def test_error_if_bucket_is_empty(self, s3_client, caplog, s3_mock_bucket): -- cgit v1.2.3 From 053e75bca8ef34a655bb4afda5f479f112dfb002 Mon Sep 17 00:00:00 2001 From: Alex Schofield Date: Thu, 22 Aug 2024 12:33:00 +0100 Subject: fix: improve error handling for list_existing_s3_files and tests --- src/extract_lambda.py | 16 ++++++++++------ tests/test_extract_lambda.py | 10 ++++++++-- 2 files changed, 18 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/extract_lambda.py b/src/extract_lambda.py index 874098b..b20c99d 100644 --- a/src/extract_lambda.py +++ b/src/extract_lambda.py @@ -118,15 +118,16 @@ def list_existing_s3_files(bucket_name=None, client=None): results of listing the contents of the s3 bucket, then returns the populated dictionary """ - if client is None: - client = boto3.client("s3") - if bucket_name is None: - bucket_name = extract_bucket(client) logging.info("Listing existing S3 files") existing_files = {} try: + if client is None: + client = boto3.client("s3") + if bucket_name is None: + bucket_name = extract_bucket(client) + response = client.list_objects_v2(Bucket=bucket_name) if "Contents" in response: @@ -142,8 +143,11 @@ def list_existing_s3_files(bucket_name=None, client=None): logger.error("The bucket is empty") return None - except ClientError as e: - logger.error(f"Error listing S3 objects: {e}") + except ValueError as ve: + logger.error(f"Error listing S3 objects: {ve}") + raise + except ClientError as ce: + logger.error(f"Error listing S3 objects: {ce}") return existing_files diff --git a/tests/test_extract_lambda.py b/tests/test_extract_lambda.py index bba433c..8fa0e88 100644 --- a/tests/test_extract_lambda.py +++ b/tests/test_extract_lambda.py @@ -195,8 +195,14 @@ class TestListExistingS3Files: logger.info("Testing now.") caplog.set_level(logging.ERROR) - with pytest.raises(ValueError, match="No extract_bucket found"): - list_existing_s3_files(client=s3_client) + # Mock the extract_bucket function to raise a ValueError! + with patch( + "src.extract_lambda.extract_bucket", + side_effect=ValueError("No extract_bucket found"), + ): + with pytest.raises(ValueError, match="No extract_bucket found"): + list_existing_s3_files(client=s3_client) + assert "Error listing S3 objects" in caplog.text def test_error_if_bucket_is_empty(self, s3_client, caplog, s3_mock_bucket): -- cgit v1.2.3