[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1031: MINIFICPP-1476 Improve parallel S3 request handling in S3 processors

[GitBox <gi...@apache.org>]

fgerlits commented on a change in pull request #1031:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1031#discussion_r608776773



##########
File path: extensions/aws/processors/DeleteS3Object.h
##########
@@ -65,6 +65,11 @@ class DeleteS3Object : public S3Processor {
   explicit DeleteS3Object(const std::string& name, const minifi::utils::Identifier& uuid, std::unique_ptr<aws::s3::S3RequestSender> s3_request_sender)
     : S3Processor(name, uuid, logging::LoggerFactory<DeleteS3Object>::getLogger(), std::move(s3_request_sender)) {
   }
+
+  minifi::utils::optional<aws::s3::DeleteObjectRequestParameters> buildDeleteS3RequestParams(
+    const std::shared_ptr<core::ProcessContext> &context,
+    const std::shared_ptr<core::FlowFile> &flow_file,
+    const CommonProperties &common_properties);

Review comment:
       minor, but this method could be `const` (also in the other operations)

##########
File path: extensions/aws/processors/PutS3Object.cpp
##########
@@ -206,7 +206,8 @@ minifi::utils::optional<aws::s3::PutObjectRequestParameters> PutS3Object::buildP
     const std::shared_ptr<core::ProcessContext> &context,
     const std::shared_ptr<core::FlowFile> &flow_file,
     const CommonProperties &common_properties) {
-  aws::s3::PutObjectRequestParameters params;
+  aws::s3::PutObjectRequestParameters params(common_properties.credentials, client_config_);
+  setClientConfig(params.client_config, common_properties);

Review comment:
       Just a suggestion, but if you made `setClientConfig()` a member of `RequestParameters` instead of a static function, then we could write `params.setClientConfig(common_properties)`, which would make it clearer (to me) that we are modifying the `params` object.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org