From cd3a6e26d45214dc36d8638c5376a587d584f438 Mon Sep 17 00:00:00 2001 From: fatimarahman Date: Tue, 21 Jul 2026 13:58:11 -0400 Subject: [PATCH 1/3] Allow user to specify encoding --- CHANGELOG.md | 3 +++ src/nypl_py_utils/classes/s3_client.py | 37 ++++++++++++++++---------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbdc9fb..16e8c1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,7 @@ # Changelog +## v1.12.2 7/21/26 +- Enable S3 client to upload files in binary format if needed + ## v1.12.1 6/18/26 - Remove Azure client encoding and decoding diff --git a/src/nypl_py_utils/classes/s3_client.py b/src/nypl_py_utils/classes/s3_client.py index c1b7e9d..e4ae895 100644 --- a/src/nypl_py_utils/classes/s3_client.py +++ b/src/nypl_py_utils/classes/s3_client.py @@ -16,13 +16,14 @@ class S3Client: """ def __init__(self, bucket, resource=None): - self.logger = create_log('s3_client') + self.logger = create_log("s3_client") self.bucket = bucket self.resource = resource try: self.s3_client = boto3.client( - 's3', region_name=os.environ.get('AWS_REGION', 'us-east-1')) + "s3", region_name=os.environ.get("AWS_REGION", "us-east-1") + ) except ClientError as e: error_msg = f'Could not create S3 client: {e}' self.logger.error(error_msg) @@ -34,7 +35,7 @@ def close(self): def fetch_cache(self): """Fetches a JSON file from S3 and returns the resulting dictionary""" self.logger.info( - f'Fetching {self.resource} from S3 bucket {self.bucket}') + f"Fetching {self.resource} from S3 bucket {self.bucket}") try: output_stream = BytesIO() self.s3_client.download_fileobj( @@ -42,27 +43,29 @@ def fetch_cache(self): return json.loads(output_stream.getvalue()) except ClientError as e: error_msg = ( - f'Error retrieving {self.resource} from S3 bucket ' - f'{self.bucket}: {e}') + f"Error retrieving {self.resource} from S3 bucket " + f"{self.bucket}: {e}" + ) self.logger.error(error_msg) raise S3ClientError(error_msg) from None def set_cache(self, state): """Writes a dictionary to JSON and uploads the resulting file to S3""" self.logger.info( - f'Setting {self.resource} in S3 bucket {self.bucket} to {state}') + f"Setting {self.resource} in S3 bucket {self.bucket} to {state}" + ) try: input_stream = BytesIO(json.dumps(state).encode()) self.s3_client.upload_fileobj( input_stream, self.bucket, self.resource) except ClientError as e: error_msg = ( - f'Error uploading {self.resource} to S3 bucket ' - f'{self.bucket}: {e}') + f"Error uploading {self.resource} to S3 bucket " + f"{self.bucket}: {e}") self.logger.error(error_msg) raise S3ClientError(error_msg) from None - def upload_file(self, content, file_path): + def upload_file(self, content, file_path, encode=True): """ Writes an arbitrary file to S3. Note that this will overwrite any existing file with the same name. @@ -74,16 +77,22 @@ def upload_file(self, content, file_path): file_path: str The full path of the file that should be written not including the bucket. Example: "subdirectory/example_file.csv" + encode: bool, optional + Whether to encode the content as utf-8 before writing. Default is + True. If False, the content should be provided in bytes (e.g. + in parquet, PDF, or image form) """ - self.logger.info( - f'Writing {file_path} in S3 bucket {self.bucket}') + self.logger.info(f"Writing {file_path} in S3 bucket {self.bucket}") try: - input_stream = BytesIO(content.encode()) + if encode: + content = content.encode("utf-8") + input_stream = BytesIO(content) self.s3_client.upload_fileobj(input_stream, self.bucket, file_path) except ClientError as e: error_msg = ( - f'Error uploading {file_path} to S3 bucket ' - f'{self.bucket}: {e}') + f"Error uploading {file_path} to S3 bucket " + f"{self.bucket}: {e}" + ) self.logger.error(error_msg) raise S3ClientError(error_msg) from None From 36266fb1fea6690c78e099931a37f26d0752366f Mon Sep 17 00:00:00 2001 From: fatimarahman Date: Tue, 21 Jul 2026 14:07:36 -0400 Subject: [PATCH 2/3] Add double quotes and update project version --- pyproject.toml | 2 +- tests/test_s3_client.py | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index f60f648..b92b5b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "nypl_py_utils" -version = "1.12.1" +version = "1.12.2" authors = [ { name="Aaron Friedman", email="aaronfriedman@nypl.org" }, ] diff --git a/tests/test_s3_client.py b/tests/test_s3_client.py index cdc8293..4014e2b 100644 --- a/tests/test_s3_client.py +++ b/tests/test_s3_client.py @@ -3,20 +3,20 @@ from nypl_py_utils.classes.s3_client import S3Client -_TEST_STATE = {'key1': 'val1', 'key2': 'val2', 'key3': 'val3'} +_TEST_STATE = {"key1": "val1", "key2": "val2", "key3": "val3"} class TestS3Client: @pytest.fixture def test_instance(self, mocker): - mocker.patch('boto3.client') - return S3Client('test_s3_bucket', 'test_s3_resource') + mocker.patch("boto3.client") + return S3Client("test_s3_bucket", "test_s3_resource") def test_fetch_cache(self, test_instance): def mock_download(bucket, resource, stream): - assert bucket == 'test_s3_bucket' - assert resource == 'test_s3_resource' + assert bucket == "test_s3_bucket" + assert resource == "test_s3_resource" stream.write(json.dumps(_TEST_STATE).encode()) test_instance.s3_client.download_fileobj.side_effect = mock_download @@ -26,12 +26,12 @@ def test_set_cache(self, test_instance): test_instance.set_cache(_TEST_STATE) arguments = test_instance.s3_client.upload_fileobj.call_args.args assert arguments[0].getvalue() == json.dumps(_TEST_STATE).encode() - assert arguments[1] == 'test_s3_bucket' - assert arguments[2] == 'test_s3_resource' + assert arguments[1] == "test_s3_bucket" + assert arguments[2] == "test_s3_resource" def test_upload_file(self, test_instance): - test_instance.upload_file('test_content', 'test_filename.txt') + test_instance.upload_file("test_content", "test_filename.txt") arguments = test_instance.s3_client.upload_fileobj.call_args.args - assert arguments[0].getvalue() == b'test_content' - assert arguments[1] == 'test_s3_bucket' - assert arguments[2] == 'test_filename.txt' + assert arguments[0].getvalue() == b"test_content" + assert arguments[1] == "test_s3_bucket" + assert arguments[2] == "test_filename.txt" From 09a385228c7b8c33134e8c630c2dc185aa86d79e Mon Sep 17 00:00:00 2001 From: fatimarahman Date: Tue, 21 Jul 2026 14:49:25 -0400 Subject: [PATCH 3/3] Remove encode param --- src/nypl_py_utils/classes/s3_client.py | 16 ++++++---------- tests/test_s3_client.py | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/nypl_py_utils/classes/s3_client.py b/src/nypl_py_utils/classes/s3_client.py index e4ae895..9dab536 100644 --- a/src/nypl_py_utils/classes/s3_client.py +++ b/src/nypl_py_utils/classes/s3_client.py @@ -65,29 +65,25 @@ def set_cache(self, state): self.logger.error(error_msg) raise S3ClientError(error_msg) from None - def upload_file(self, content, file_path, encode=True): + def upload_file(self, content, file_path): """ Writes an arbitrary file to S3. Note that this will overwrite any existing file with the same name. Parameters ---------- - content: str - The string that should be written to the file. Must be utf-8. + content: str | bytes + The data that should be written to the file file_path: str The full path of the file that should be written not including the bucket. Example: "subdirectory/example_file.csv" - encode: bool, optional - Whether to encode the content as utf-8 before writing. Default is - True. If False, the content should be provided in bytes (e.g. - in parquet, PDF, or image form) """ self.logger.info(f"Writing {file_path} in S3 bucket {self.bucket}") try: - if encode: + if isinstance(content, str): content = content.encode("utf-8") - input_stream = BytesIO(content) - self.s3_client.upload_fileobj(input_stream, self.bucket, file_path) + self.s3_client.upload_fileobj( + BytesIO(content), self.bucket, file_path) except ClientError as e: error_msg = ( f"Error uploading {file_path} to S3 bucket " diff --git a/tests/test_s3_client.py b/tests/test_s3_client.py index 4014e2b..d82541f 100644 --- a/tests/test_s3_client.py +++ b/tests/test_s3_client.py @@ -29,9 +29,22 @@ def test_set_cache(self, test_instance): assert arguments[1] == "test_s3_bucket" assert arguments[2] == "test_s3_resource" - def test_upload_file(self, test_instance): + def test_upload_file_encoded(self, test_instance, mocker): test_instance.upload_file("test_content", "test_filename.txt") arguments = test_instance.s3_client.upload_fileobj.call_args.args - assert arguments[0].getvalue() == b"test_content" + expected_content = "test_content".encode("utf-8") + + # check that the content is encoded as utf-8 before being sent to S3 + assert arguments[0].getvalue() == expected_content assert arguments[1] == "test_s3_bucket" assert arguments[2] == "test_filename.txt" + + def test_upload_file_binary(self, test_instance): + binary_content = b"PAR1\x00\x01\xff\xfe" + test_instance.upload_file(binary_content, "test_filename.parquet") + arguments = test_instance.s3_client.upload_fileobj.call_args.args + + # check bytes aren't re-encoded; they should reach S3 as-is + assert arguments[0].getvalue() == binary_content + assert arguments[1] == "test_s3_bucket" + assert arguments[2] == "test_filename.parquet"