szaszm commented on code in PR #2157:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2157#discussion_r3065316890
##########
extensions/aws/s3/MultipartUploadStateStorage.cpp:
##########
@@ -24,6 +24,9 @@ namespace org::apache::nifi::minifi::aws::s3 {
void MultipartUploadStateStorage::storeState(const std::string& bucket, const
std::string& key, const MultipartUploadState& state) {
std::unordered_map<std::string, std::string> stored_state;
std::lock_guard<std::mutex> lock(state_manager_mutex_);
+ if (!state_manager_) {
+ return;
Review Comment:
Shouldn't this be an error, like an exception?
##########
extension-framework/include/utils/ListingStateManager.h:
##########
@@ -49,7 +49,7 @@ struct ListingState {
class ListingStateManager {
public:
- explicit ListingStateManager(core::StateManager* state_manager)
+ explicit ListingStateManager(gsl::not_null<core::StateManager*>
state_manager)
: state_manager_(state_manager) {
Review Comment:
In parameters, I'd use references instead of not_null pointers, but in data
members, I'd keep the not_null pointer, because members generally shouldn't be
const.
```suggestion
explicit ListingStateManager(core::StateManager& state_manager)
: state_manager_(&state_manager) {
```
##########
extensions/aws/s3/S3Wrapper.cpp:
##########
@@ -463,10 +465,11 @@ bool S3Wrapper::abortMultipartUpload(const
AbortMultipartUploadRequestParameters
}
void S3Wrapper::ageOffLocalS3MultipartUploadStates(std::chrono::milliseconds
multipart_upload_max_age_threshold) {
+ gsl_Expects(multipart_upload_storage_);
multipart_upload_storage_->removeAgedStates(multipart_upload_max_age_threshold);
}
-void
S3Wrapper::initializeMultipartUploadStateStorage(gsl::not_null<minifi::core::StateManager*>
state_manager) {
+void
S3Wrapper::initializeMultipartUploadStateStorage(gsl::not_null<core::StateManager*>
state_manager) {
multipart_upload_storage_ =
std::make_unique<MultipartUploadStateStorage>(state_manager);
Review Comment:
```suggestion
void S3Wrapper::initializeMultipartUploadStateStorage(core::StateManager&
state_manager) {
multipart_upload_storage_ =
std::make_unique<MultipartUploadStateStorage>(state_manager);
```
--
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]
