szaszm commented on code in PR #1586:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1586#discussion_r1232540073
##########
extensions/aws/s3/S3Wrapper.cpp:
##########
@@ -75,42 +69,155 @@ std::string
S3Wrapper::getEncryptionString(Aws::S3::Model::ServerSideEncryption
return "";
}
-std::optional<PutObjectResult> S3Wrapper::putObject(const
PutObjectRequestParameters& put_object_params, const
std::shared_ptr<Aws::IOStream>& data_stream) {
- Aws::S3::Model::PutObjectRequest request;
- request.SetBucket(put_object_params.bucket);
- request.SetKey(put_object_params.object_key);
-
request.SetStorageClass(STORAGE_CLASS_MAP.at(put_object_params.storage_class));
-
request.SetServerSideEncryption(SERVER_SIDE_ENCRYPTION_MAP.at(put_object_params.server_side_encryption));
- request.SetContentType(put_object_params.content_type);
- request.SetMetadata(put_object_params.user_metadata_map);
+std::shared_ptr<Aws::StringStream> S3Wrapper::readFlowFileStream(const
std::shared_ptr<io::InputStream>& stream, uint64_t read_limit, uint64_t&
read_size_out) {
+ std::vector<std::byte> buffer;
+ buffer.resize(BUFFER_SIZE);
Review Comment:
With a sufficiently large buffer size, this risks stack overflow. Not sure
about the stack sizes in practice, but I'd only expect this to be a problem on
embedded devices with the current 4K buffer size.
Since we're doing IO in this function, the overhead of an extra allocation
is probably insignificant compared to any IO overhead. But there is no harm in
changing it. I'm neutral on this, just wanted to share some thoughts and add
nuance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
