Respect file extension setting for S3 destinations in Firehose provider #10651
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
There was a problem hiding this comment.
Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.
|
I have read the CLA Document and I hereby sign the CLA |
pinzon
added
the
semver: patch
|
Hello @ozgenbaris1 , thank you for taking the initiative to address an issue that you have encountered. In order to proceed, it is important for the solution to include an integration test. |
The Firehose delivery stream implementation was not considering the file extension setting when the target was defined as an S3 bucket. This fix ensures that the specified file extension is appended to the S3 object's key when writing data. The `_get_s3_object_path` method now accepts the `file_extension` parameter to accommodate this enhancement.
ozgenbaris1
force-pushed
the
firehose-file-extension
branch
from
0162d87
to
199af06
Compare
|
hello @pinzon! just added an integration test. can you review? |
| @@ -532,3 +532,44 @@ def test_kinesis_firehose_kinesis_as_source_multiple_delivery_streams( | |||
| ] | |||
| ) | |||
| snapshot.match("kinesis-event-stream-multiple-delivery-streams", s3_data) | |||
|
|
|||
| @markers.aws.unknown | |||
There was a problem hiding this comment.
we're not accepting any more unknown tests. Please use the TEST_TARGET env variable to run agains the AWS. This will generate and extra file that you need to also submit in this pr. Also change it to @markers.aws.validated once you confirm that the test passes with AWS . Please check these docs for more info.
| Record=record, | ||
| ) | ||
|
|
||
| s3_objects = list_s3_objects(s3_bucket, timeout=300) |
There was a problem hiding this comment.
| s3_objects = list_s3_objects(s3_bucket, timeout=300) | |
| s3_objects = list_s3_objects(s3_bucket, timeout= 300 if is_aws_cloud() else 60) |
We should consider if the test is running in AWS
Changes
The Firehose delivery stream implementation was not considering the file extension setting when the target was defined as an S3 bucket. This fix ensures that the specified file extension is appended to the S3 object's key when writing data. The
_get_s3_object_pathmethod now accepts thefile_extensionparameter to accommodate this enhancement.This change introduces support for file extensions by extracting the FileExtension parameter from the S3 destination description. Now, if a file extension is provided, it will be appended to the S3 object's key, ensuring that files are created with the correct extensions, improving flexibility for users relying on file extensions for downstream processing or identification of data within S3 buckets.
Motivation
resolves #10650