szaszm commented on code in PR #1586:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1586#discussion_r1231981938
##########
extensions/aws/processors/PutS3Object.cpp:
##########
@@ -77,7 +78,31 @@ void PutS3Object::onSchedule(const
std::shared_ptr<core::ProcessContext> &contex
use_virtual_addressing_ = !*use_path_style_access;
}
+ context->getProperty(MultipartThreshold.getName(), multipart_threshold_);
+ if (multipart_threshold_ > getMaxUploadSize() || multipart_threshold_ <
getMinPartSize()) {
+ throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Multipart Threshold is not
between the valid 5MB and 5GB range!");
+ }
+ logger_->log_debug("PutS3Object: Multipart Threshold %" PRIu64,
multipart_threshold_);
+ context->getProperty(MultipartPartSize.getName(), multipart_size_);
+ if (multipart_size_ > getMaxUploadSize() || multipart_size_ <
getMinPartSize()) {
+ throw Exception(PROCESS_SCHEDULE_EXCEPTION, "Multipart Part Size is not
between the valid 5MB and 5GB range!");
+ }
+ logger_->log_debug("PutS3Object: Multipart Size %" PRIu64, multipart_size_);
+
+
+ multipart_upload_ageoff_interval_ =
minifi::utils::getRequiredPropertyOrThrow<core::TimePeriodValue>(*context,
MultipartUploadAgeOffInterval.getName()).getMilliseconds();
+ logger_->log_debug("PutS3Object: Multipart Upload Ageoff Interval %" PRIu64
" ms", multipart_upload_ageoff_interval_.count());
+
+ multipart_upload_max_age_threshold_ =
minifi::utils::getRequiredPropertyOrThrow<core::TimePeriodValue>(*context,
MultipartUploadMaxAgeThreshold.getName()).getMilliseconds();
+ logger_->log_debug("PutS3Object: Multipart Upload Max Age Threshold %"
PRIu64 " ms", multipart_upload_max_age_threshold_.count());
+
fillUserMetadata(context);
+
+ std::string multipart_temp_dir;
+ context->getProperty(TemporaryDirectoryMultipartState.getName(),
multipart_temp_dir);
+
+
+ s3_wrapper_.initailizeMultipartUploadStateStorage(multipart_temp_dir,
getUUIDStr());
Review Comment:
I don't really like circumventing the state manager, we should aim to make
it work by extending it instead IMO. Not necessarily in this PR, but that
works, too. And I want to avoid introducing a property that will need to be
removed later, because removing a processor property may break old config.ymls
that contain it. (Unless dynamic properties are enabled, but they don't seem to
make sense here.)
I think should go with another compromise, my ideas:
1. Give up the functionality of continuing failed multipart uploads after a
restart. It could still continue if the agent wasn't restarted, but restart
would restart the upload.
2. Use a system-wide temporary directory, e.g.
/tmp/minifi-multipart-state/processor_uuid
3. Configure the multipart state dir globally in minifi.properties, because
that doesn't break configs when we drop the config property
I gravitate towards 1, but I'm interested in your viewpoint.
--
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]
