Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
szaszm closed pull request #1706: MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … URL: https://github.com/apache/nifi-minifi-cpp/pull/1706 -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
adamdebreceni commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1452390024 ## extensions/standard-processors/processors/GenerateFlowFile.h: ## @@ -70,14 +66,15 @@ class GenerateFlowFile : public core::Processor { .withDefaultValue("Binary") .build(); EXTENSIONAPI static constexpr auto UniqueFlowFiles = core::PropertyDefinitionBuilder<>::createProperty("Unique FlowFiles") - .withDescription("If true, each FlowFile that is generated will be unique. If false, a random value will be generated and all FlowFiles") + .withDescription("If true, each FlowFile that is generated will be unique. " + "If false, a random value will be generated and all FlowFiles will get the same content but this offers much higher throughput") Review Comment: as discussed the description of CustomText is clear enough on how to use that and what to expect -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
adamdebreceni commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1448697981 ## extensions/standard-processors/processors/GenerateFlowFile.h: ## @@ -70,14 +66,15 @@ class GenerateFlowFile : public core::Processor { .withDefaultValue("Binary") .build(); EXTENSIONAPI static constexpr auto UniqueFlowFiles = core::PropertyDefinitionBuilder<>::createProperty("Unique FlowFiles") - .withDescription("If true, each FlowFile that is generated will be unique. If false, a random value will be generated and all FlowFiles") + .withDescription("If true, each FlowFile that is generated will be unique. " + "If false, a random value will be generated and all FlowFiles will get the same content but this offers much higher throughput") Review Comment: yes that is what I meant, so the description is not true -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1447303271 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -57,58 +56,78 @@ void generateData(std::vector& data, bool textData = false) { } } -void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); - } +GenerateFlowFile::Mode GenerateFlowFile::getMode(bool is_unique, bool is_text, bool has_custom_text, uint64_t file_size) { + if (is_text && !is_unique && has_custom_text) +return Mode::CustomText; - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); - } + if (file_size == 0) +return Mode::Empty; - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (is_unique) { +if (is_text) + return Mode::UniqueText; +else + return Mode::UniqueByte; + } else { +if (is_text) + return Mode::NotUniqueText; +else + return Mode::NotUniqueByte; } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); +} + +void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { + bool is_text = context.getProperty(DataFormat) + | utils::transform([](const std::string& data_format) { return data_format == DATA_FORMAT_TEXT;}) + | utils::valueOrElse([]() {return false;}); + bool is_unique = context.getProperty(UniqueFlowFiles) | utils::valueOrElse([] { return true; }); + + auto custom_text_without_evaluation = context.getProperty(CustomText); + bool has_custom_text = custom_text_without_evaluation.has_value() && !custom_text_without_evaluation->empty(); + + context.getProperty(FileSize, file_size_); + context.getProperty(BatchSize, batch_size_); + + mode_ = getMode(is_unique, is_text, has_custom_text, file_size_); + + if (!isUnique(mode_)) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, isText(mode_)); } + logger_->log_trace("GenerateFlowFile is configured in {} mode", magic_enum::enum_name(mode_)); + if (mode_ != Mode::CustomText && has_custom_text) +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: Clarified this in https://github.com/apache/nifi-minifi-cpp/pull/1706/commits/59becf11fea3e4d986b11acf6e8f33765a8b3408 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -57,58 +56,78 @@ void generateData(std::vector& data, bool textData = false) { } } -void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); - } +GenerateFlowFile::Mode GenerateFlowFile::getMode(bool is_unique, bool is_text, bool has_custom_text, uint64_t file_size) { + if (is_text && !is_unique && has_custom_text) +return Mode::CustomText; - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); - } + if (file_size == 0) +return Mode::Empty; - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (is_unique) { +if (is_text) + return Mode::UniqueText; +else + return Mode::UniqueByte; + } else { +if (is_text) + return Mode::NotUniqueText; +else + return Mode::NotUniqueByte; } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); +} + +void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { + bool is_text = context.getProperty(DataFormat) + | utils::transform([](const std::string& data_format) { return data_format == DATA_FORMAT_TEXT;}) + | utils::valueOrElse([]() {return false;}); + bool is_unique = context.getProperty(UniqueFlowFiles) | utils::valueOrElse([] { return true; }); + + auto custom_text_without_evaluation = context.getProperty(CustomText); + bool has_custom_text = custom_text_without_evaluation.has_value() && !custom_text_without_evaluation->empty(); + + context.getProperty(FileSize, file_size_); + context.getProperty(BatchSize, batch_size_); + + mode_ = getMode(is_unique, is_text, has_custom_text, file_size_); + + if (!isUnique(mode_)) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, isText(mode_)); } +
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1447302157 ## extensions/standard-processors/processors/GenerateFlowFile.h: ## @@ -70,14 +66,15 @@ class GenerateFlowFile : public core::Processor { .withDefaultValue("Binary") .build(); EXTENSIONAPI static constexpr auto UniqueFlowFiles = core::PropertyDefinitionBuilder<>::createProperty("Unique FlowFiles") - .withDescription("If true, each FlowFile that is generated will be unique. If false, a random value will be generated and all FlowFiles") + .withDescription("If true, each FlowFile that is generated will be unique. " + "If false, a random value will be generated and all FlowFiles will get the same content but this offers much higher throughput") Review Comment: All flowfiles period. The description is directly from NiFi. And afaik nifi and minifi works in the same way so if this is false we only generate the date once. (except if CustomText is set then we should evaluate it once per batch) -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1447274369 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -57,58 +56,78 @@ void generateData(std::vector& data, bool textData = false) { } } -void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); - } +GenerateFlowFile::Mode GenerateFlowFile::getMode(bool is_unique, bool is_text, bool has_custom_text, uint64_t file_size) { + if (is_text && !is_unique && has_custom_text) +return Mode::CustomText; - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); - } + if (file_size == 0) +return Mode::Empty; - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (is_unique) { +if (is_text) + return Mode::UniqueText; +else + return Mode::UniqueByte; + } else { +if (is_text) + return Mode::NotUniqueText; +else + return Mode::NotUniqueByte; } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); +} + +void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { + bool is_text = context.getProperty(DataFormat) + | utils::transform([](const std::string& data_format) { return data_format == DATA_FORMAT_TEXT;}) + | utils::valueOrElse([]() {return false;}); + bool is_unique = context.getProperty(UniqueFlowFiles) | utils::valueOrElse([] { return true; }); + + auto custom_text_without_evaluation = context.getProperty(CustomText); + bool has_custom_text = custom_text_without_evaluation.has_value() && !custom_text_without_evaluation->empty(); + + context.getProperty(FileSize, file_size_); + context.getProperty(BatchSize, batch_size_); + + mode_ = getMode(is_unique, is_text, has_custom_text, file_size_); + + if (!isUnique(mode_)) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, isText(mode_)); } + logger_->log_trace("GenerateFlowFile is configured in {} mode", magic_enum::enum_name(mode_)); + if (mode_ != Mode::CustomText && has_custom_text) +logger_->log_warn("Custom Text property is set, but not used!"); +} + +// The custom text has to be reevaluated once per batch +void GenerateFlowFile::refreshNonUniqueData(core::ProcessContext& context) { + if (mode_ != Mode::CustomText) +return; std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); - if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); - return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); -} - } - - if (!uniqueFlowFile_) { -data_.resize(gsl::narrow(fileSize_)); -generateData(data_, textData_); - } + non_unique_data_.assign(custom_text.begin(), custom_text.end()); } -void GenerateFlowFile::onTrigger(core::ProcessContext&, core::ProcessSession& session) { - for (uint64_t i = 0; i < batchSize_; i++) { +void GenerateFlowFile::onTrigger(core::ProcessContext& context, core::ProcessSession& session) { + refreshNonUniqueData(context); + for (uint64_t i = 0; i < batch_size_; i++) { // For each batch -std::shared_ptr flowFile = session.create(); -if (!flowFile) { +std::shared_ptr flow_file = session.create(); +if (!flow_file) { logger_->log_error("Failed to create flowfile!"); return; } -if (uniqueFlowFile_) { - std::vector data(gsl::narrow(fileSize_)); - if (fileSize_ > 0) { -generateData(data, textData_); - } - session.writeBuffer(flowFile, data); +if (mode_ == Mode::Empty) { + // noop Review Comment: we do, the creation is in line 116 and the transfer is after all these ifs in line 130, We only skip the modification of the created empty flowfile -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
szaszm commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1446307739 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -57,58 +56,78 @@ void generateData(std::vector& data, bool textData = false) { } } -void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); - } +GenerateFlowFile::Mode GenerateFlowFile::getMode(bool is_unique, bool is_text, bool has_custom_text, uint64_t file_size) { + if (is_text && !is_unique && has_custom_text) +return Mode::CustomText; - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); - } + if (file_size == 0) +return Mode::Empty; - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (is_unique) { +if (is_text) + return Mode::UniqueText; +else + return Mode::UniqueByte; + } else { +if (is_text) + return Mode::NotUniqueText; +else + return Mode::NotUniqueByte; } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); +} + +void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { + bool is_text = context.getProperty(DataFormat) + | utils::transform([](const std::string& data_format) { return data_format == DATA_FORMAT_TEXT;}) + | utils::valueOrElse([]() {return false;}); + bool is_unique = context.getProperty(UniqueFlowFiles) | utils::valueOrElse([] { return true; }); + + auto custom_text_without_evaluation = context.getProperty(CustomText); + bool has_custom_text = custom_text_without_evaluation.has_value() && !custom_text_without_evaluation->empty(); + + context.getProperty(FileSize, file_size_); + context.getProperty(BatchSize, batch_size_); + + mode_ = getMode(is_unique, is_text, has_custom_text, file_size_); + + if (!isUnique(mode_)) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, isText(mode_)); } + logger_->log_trace("GenerateFlowFile is configured in {} mode", magic_enum::enum_name(mode_)); + if (mode_ != Mode::CustomText && has_custom_text) +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: It may be worth clarifying why it's not used, or how to get it to be used. ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -57,58 +56,78 @@ void generateData(std::vector& data, bool textData = false) { } } -void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); - } +GenerateFlowFile::Mode GenerateFlowFile::getMode(bool is_unique, bool is_text, bool has_custom_text, uint64_t file_size) { + if (is_text && !is_unique && has_custom_text) +return Mode::CustomText; - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); - } + if (file_size == 0) +return Mode::Empty; - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (is_unique) { +if (is_text) + return Mode::UniqueText; +else + return Mode::UniqueByte; + } else { +if (is_text) + return Mode::NotUniqueText; +else + return Mode::NotUniqueByte; } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); +} + +void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { + bool is_text = context.getProperty(DataFormat) + | utils::transform([](const std::string& data_format) { return data_format == DATA_FORMAT_TEXT;}) + | utils::valueOrElse([]() {return false;}); + bool is_unique = context.getProperty(UniqueFlowFiles) | utils::valueOrElse([] { return true; }); + + auto custom_text_without_evaluation = context.getProperty(CustomText); + bool has_custom_text = custom_text_without_evaluation.has_value() && !custom_text_without_evaluation->empty(); + + context.getProperty(FileSize, file_size_); + context.getProperty(BatchSize, batch_size_); + + mode_ = getMode(is_unique, is_text, has_custom_text, file_size_); + + if (!isUnique(mode_)) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, isText(mode_)); } + logger_->log_trace("GenerateFlowFile is configured in {} mode",
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
adamdebreceni commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1444645787 ## extensions/standard-processors/processors/GenerateFlowFile.h: ## @@ -70,14 +66,15 @@ class GenerateFlowFile : public core::Processor { .withDefaultValue("Binary") .build(); EXTENSIONAPI static constexpr auto UniqueFlowFiles = core::PropertyDefinitionBuilder<>::createProperty("Unique FlowFiles") - .withDescription("If true, each FlowFile that is generated will be unique. If false, a random value will be generated and all FlowFiles") + .withDescription("If true, each FlowFile that is generated will be unique. " + "If false, a random value will be generated and all FlowFiles will get the same content but this offers much higher throughput") Review Comment: all FlowFiles in a batch? -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
fgerlits commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1442836812 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -57,58 +56,78 @@ void generateData(std::vector& data, bool textData = false) { } } -void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); - } +GenerateFlowFile::Mode GenerateFlowFile::getMode(bool is_unique, bool is_text, bool has_custom_text, uint64_t file_size) { + if (is_text && !is_unique && has_custom_text) +return Mode::CustomText; - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); - } + if (file_size == 0) +return Mode::Empty; - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (is_unique) { +if (is_text) + return Mode::UniqueText; +else + return Mode::UniqueByte; + } else { +if (is_text) + return Mode::NotUniqueText; +else + return Mode::NotUniqueByte; } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); +} + +void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { + bool is_text = context.getProperty(DataFormat) + | utils::transform([](const std::string& data_format) { return data_format == DATA_FORMAT_TEXT;}) + | utils::valueOrElse([]() {return false;}); + bool is_unique = context.getProperty(UniqueFlowFiles) | utils::valueOrElse([] { return true; }); + + auto custom_text_without_evaluation = context.getProperty(CustomText); + bool has_custom_text = custom_text_without_evaluation.has_value() && !custom_text_without_evaluation->empty(); + + context.getProperty(FileSize, file_size_); + context.getProperty(BatchSize, batch_size_); + + mode_ = getMode(is_unique, is_text, has_custom_text, file_size_); + + if (!isUnique(mode_)) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, isText(mode_)); } + logger_->log_trace("GenerateFlowFile is configure in {} mode", magic_enum::enum_name(mode_)); Review Comment: nitpicking: ```suggestion logger_->log_trace("GenerateFlowFile is configured in {} mode", magic_enum::enum_name(mode_)); ``` -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1442772416 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -58,57 +58,69 @@ void generateData(std::vector& data, bool textData = false) { } void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); + if (context.getProperty(FileSize.name, file_size_)) { +logger_->log_trace("File size is configured to be {}", file_size_); } - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); + if (context.getProperty(BatchSize.name, batch_size_)) { +logger_->log_trace("Batch size is configured to be {}", batch_size_); } - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (auto data_format = context.getProperty(DataFormat)) { +textData_ = (*data_format == DATA_FORMAT_TEXT); } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); + if (context.getProperty(UniqueFlowFiles.name, unique_flow_file_)) { +logger_->log_trace("Unique Flow files is configured to be {}", unique_flow_file_); } std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); } +logger_->log_warn("Custom Text property is set, but not used!"); } - if (!uniqueFlowFile_) { -data_.resize(gsl::narrow(fileSize_)); -generateData(data_, textData_); + if (!unique_flow_file_) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, textData_); + } +} + +// If the Data Format is text and if Unique FlowFiles is false, the custom text has to be evaluated once per batch +void GenerateFlowFile::regenerateNonUniqueData(core::ProcessContext& context) { + std::string custom_text; + context.getProperty(CustomText, custom_text, nullptr); + if (!custom_text.empty()) { +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); + return; +} +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: Added an other test that covers this last use case https://github.com/apache/nifi-minifi-cpp/pull/1706/commits/13f0f74c7e46e4228c0b1cab7381a3b8416094ec -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1442770133 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -58,57 +58,69 @@ void generateData(std::vector& data, bool textData = false) { } void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); + if (context.getProperty(FileSize.name, file_size_)) { +logger_->log_trace("File size is configured to be {}", file_size_); } - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); + if (context.getProperty(BatchSize.name, batch_size_)) { +logger_->log_trace("Batch size is configured to be {}", batch_size_); } - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (auto data_format = context.getProperty(DataFormat)) { +textData_ = (*data_format == DATA_FORMAT_TEXT); } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); + if (context.getProperty(UniqueFlowFiles.name, unique_flow_file_)) { +logger_->log_trace("Unique Flow files is configured to be {}", unique_flow_file_); } std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); } +logger_->log_warn("Custom Text property is set, but not used!"); } - if (!uniqueFlowFile_) { -data_.resize(gsl::narrow(fileSize_)); -generateData(data_, textData_); + if (!unique_flow_file_) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, textData_); + } +} + +// If the Data Format is text and if Unique FlowFiles is false, the custom text has to be evaluated once per batch +void GenerateFlowFile::regenerateNonUniqueData(core::ProcessContext& context) { + std::string custom_text; + context.getProperty(CustomText, custom_text, nullptr); + if (!custom_text.empty()) { +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); + return; +} +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: I've refactored a little bit so Its easier to see what determines the output mode Made sure that we dont break compatibility so CustomText "" is the same as no CustomText, but if CustomText were to evaluate to "" we would create an empty FF. https://github.com/apache/nifi-minifi-cpp/pull/1706/commits/626d69c49f1c2ba04ccbd36d3ceba1fff6062e76 -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
fgerlits commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1441765143 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -58,57 +58,69 @@ void generateData(std::vector& data, bool textData = false) { } void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); + if (context.getProperty(FileSize.name, file_size_)) { +logger_->log_trace("File size is configured to be {}", file_size_); } - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); + if (context.getProperty(BatchSize.name, batch_size_)) { +logger_->log_trace("Batch size is configured to be {}", batch_size_); } - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (auto data_format = context.getProperty(DataFormat)) { +textData_ = (*data_format == DATA_FORMAT_TEXT); } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); + if (context.getProperty(UniqueFlowFiles.name, unique_flow_file_)) { +logger_->log_trace("Unique Flow files is configured to be {}", unique_flow_file_); } std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); } +logger_->log_warn("Custom Text property is set, but not used!"); } - if (!uniqueFlowFile_) { -data_.resize(gsl::narrow(fileSize_)); -generateData(data_, textData_); + if (!unique_flow_file_) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, textData_); + } +} + +// If the Data Format is text and if Unique FlowFiles is false, the custom text has to be evaluated once per batch +void GenerateFlowFile::regenerateNonUniqueData(core::ProcessContext& context) { + std::string custom_text; + context.getProperty(CustomText, custom_text, nullptr); + if (!custom_text.empty()) { +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); + return; +} +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: Yes, good point: we used to treat a `Custom Text` set to empty the same as if it wasn't set at all, which is consistent with most other processors, and we probably don't want to change that. I'm OK with either remembering in `onSchedule` if `Custom Text` was empty, and using that info here, or to just treat a `Custom Text` expanded to empty as if it was empty originally -- whichever you prefer. A non-empty `Custom Text` expanding to empty is an edge case which may never happen in real life. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1441763548 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -58,57 +58,69 @@ void generateData(std::vector& data, bool textData = false) { } void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); + if (context.getProperty(FileSize.name, file_size_)) { +logger_->log_trace("File size is configured to be {}", file_size_); } - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); + if (context.getProperty(BatchSize.name, batch_size_)) { +logger_->log_trace("Batch size is configured to be {}", batch_size_); } - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (auto data_format = context.getProperty(DataFormat)) { +textData_ = (*data_format == DATA_FORMAT_TEXT); } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); + if (context.getProperty(UniqueFlowFiles.name, unique_flow_file_)) { +logger_->log_trace("Unique Flow files is configured to be {}", unique_flow_file_); } std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); } +logger_->log_warn("Custom Text property is set, but not used!"); } - if (!uniqueFlowFile_) { -data_.resize(gsl::narrow(fileSize_)); -generateData(data_, textData_); + if (!unique_flow_file_) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, textData_); + } +} + +// If the Data Format is text and if Unique FlowFiles is false, the custom text has to be evaluated once per batch +void GenerateFlowFile::regenerateNonUniqueData(core::ProcessContext& context) { + std::string custom_text; + context.getProperty(CustomText, custom_text, nullptr); + if (!custom_text.empty()) { +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); + return; +} +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: https://github.com/apache/nifi-minifi-cpp/pull/1706/commits/89c76dc4a13ab07d6a73385f5eda37aafc2dc31b#diff-45136a0df5089a3527479406b275001e601b5a9e02fa3a345be89e18e141b03bR96-R98 I've also fixed the cut off description of Unique FlowFiles https://github.com/apache/nifi-minifi-cpp/pull/1706/commits/89c76dc4a13ab07d6a73385f5eda37aafc2dc31b#diff-fd2410931e7fdc4bf8b3ce23f5f7a27c7aacdf9337320626d86d806448c90b9bR1138 And simplified the tests so they don't rely on PutFile (also speeds things up a bit because we dont have to wait for filesystem io) -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1441713014 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -58,57 +58,69 @@ void generateData(std::vector& data, bool textData = false) { } void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); + if (context.getProperty(FileSize.name, file_size_)) { +logger_->log_trace("File size is configured to be {}", file_size_); } - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); + if (context.getProperty(BatchSize.name, batch_size_)) { +logger_->log_trace("Batch size is configured to be {}", batch_size_); } - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (auto data_format = context.getProperty(DataFormat)) { +textData_ = (*data_format == DATA_FORMAT_TEXT); } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); + if (context.getProperty(UniqueFlowFiles.name, unique_flow_file_)) { +logger_->log_trace("Unique Flow files is configured to be {}", unique_flow_file_); } std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); } +logger_->log_warn("Custom Text property is set, but not used!"); } - if (!uniqueFlowFile_) { -data_.resize(gsl::narrow(fileSize_)); -generateData(data_, textData_); + if (!unique_flow_file_) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, textData_); + } +} + +// If the Data Format is text and if Unique FlowFiles is false, the custom text has to be evaluated once per batch +void GenerateFlowFile::regenerateNonUniqueData(core::ProcessContext& context) { + std::string custom_text; + context.getProperty(CustomText, custom_text, nullptr); + if (!custom_text.empty()) { +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); + return; +} +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: Nifi seems to work similarly: - CustomText not set -> 10B random text - CustomText set to ${something_that_doesnt_exists} -> empty ff One thing thats different that nifi doesnt allow empty string for customtext -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1441713014 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -58,57 +58,69 @@ void generateData(std::vector& data, bool textData = false) { } void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); + if (context.getProperty(FileSize.name, file_size_)) { +logger_->log_trace("File size is configured to be {}", file_size_); } - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); + if (context.getProperty(BatchSize.name, batch_size_)) { +logger_->log_trace("Batch size is configured to be {}", batch_size_); } - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (auto data_format = context.getProperty(DataFormat)) { +textData_ = (*data_format == DATA_FORMAT_TEXT); } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); + if (context.getProperty(UniqueFlowFiles.name, unique_flow_file_)) { +logger_->log_trace("Unique Flow files is configured to be {}", unique_flow_file_); } std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); } +logger_->log_warn("Custom Text property is set, but not used!"); } - if (!uniqueFlowFile_) { -data_.resize(gsl::narrow(fileSize_)); -generateData(data_, textData_); + if (!unique_flow_file_) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, textData_); + } +} + +// If the Data Format is text and if Unique FlowFiles is false, the custom text has to be evaluated once per batch +void GenerateFlowFile::regenerateNonUniqueData(core::ProcessContext& context) { + std::string custom_text; + context.getProperty(CustomText, custom_text, nullptr); + if (!custom_text.empty()) { +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); + return; +} +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: Nifi seem to work similarly: - CustomText not set -> 10B random text - CustomText -> ${something_that_doesnt_exists} One thing thats different that nifi doesnt allow empty string for customtext ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -58,57 +58,69 @@ void generateData(std::vector& data, bool textData = false) { } void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); + if (context.getProperty(FileSize.name, file_size_)) { +logger_->log_trace("File size is configured to be {}", file_size_); } - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); + if (context.getProperty(BatchSize.name, batch_size_)) { +logger_->log_trace("Batch size is configured to be {}", batch_size_); } - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (auto data_format = context.getProperty(DataFormat)) { +textData_ = (*data_format == DATA_FORMAT_TEXT); } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); + if (context.getProperty(UniqueFlowFiles.name, unique_flow_file_)) { +logger_->log_trace("Unique Flow files is configured to be {}", unique_flow_file_); } std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); } +logger_->log_warn("Custom Text property is set, but not used!"); } - if (!uniqueFlowFile_) { -
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1441709898 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -58,57 +58,69 @@ void generateData(std::vector& data, bool textData = false) { } void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); + if (context.getProperty(FileSize.name, file_size_)) { +logger_->log_trace("File size is configured to be {}", file_size_); } - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); + if (context.getProperty(BatchSize.name, batch_size_)) { +logger_->log_trace("Batch size is configured to be {}", batch_size_); } - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (auto data_format = context.getProperty(DataFormat)) { +textData_ = (*data_format == DATA_FORMAT_TEXT); } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); + if (context.getProperty(UniqueFlowFiles.name, unique_flow_file_)) { +logger_->log_trace("Unique Flow files is configured to be {}", unique_flow_file_); } std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); } +logger_->log_warn("Custom Text property is set, but not used!"); } - if (!uniqueFlowFile_) { -data_.resize(gsl::narrow(fileSize_)); -generateData(data_, textData_); + if (!unique_flow_file_) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, textData_); + } +} + +// If the Data Format is text and if Unique FlowFiles is false, the custom text has to be evaluated once per batch +void GenerateFlowFile::regenerateNonUniqueData(core::ProcessContext& context) { + std::string custom_text; + context.getProperty(CustomText, custom_text, nullptr); + if (!custom_text.empty()) { +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); + return; +} +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: The last suggestion would remove the possibility to use GenerateFlowFiles with non unique randomly generated set length data. (UniqueFlowFiles -> false, FileSize -> 10B, without CustomData, would generate empty flowfiles) How about we make a distinction between CustomText not set and CustomText evaluated to be empty? ``` void GenerateFlowFile::regenerateNonUniqueData(core::ProcessContext& context) { if (!textData_ || unique_flow_file_) return; if (auto custom_text = context.getProperty(CustomText, nullptr)) { non_unique_data_.assign(custom_text->begin(), custom_text->end()); } } ``` -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
fgerlits commented on code in PR #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706#discussion_r1439395796 ## extensions/standard-processors/processors/GenerateFlowFile.cpp: ## @@ -58,57 +58,69 @@ void generateData(std::vector& data, bool textData = false) { } void GenerateFlowFile::onSchedule(core::ProcessContext& context, core::ProcessSessionFactory&) { - if (context.getProperty(FileSize.name, fileSize_)) { -logger_->log_trace("File size is configured to be {}", fileSize_); + if (context.getProperty(FileSize.name, file_size_)) { +logger_->log_trace("File size is configured to be {}", file_size_); } - if (context.getProperty(BatchSize.name, batchSize_)) { -logger_->log_trace("Batch size is configured to be {}", batchSize_); + if (context.getProperty(BatchSize.name, batch_size_)) { +logger_->log_trace("Batch size is configured to be {}", batch_size_); } - std::string value; - if (context.getProperty(DataFormat.name, value)) { -textData_ = (value == GenerateFlowFile::DATA_FORMAT_TEXT); + if (auto data_format = context.getProperty(DataFormat)) { +textData_ = (*data_format == DATA_FORMAT_TEXT); } - if (context.getProperty(UniqueFlowFiles.name, uniqueFlowFile_)) { -logger_->log_trace("Unique Flow files is configured to be {}", uniqueFlowFile_); + if (context.getProperty(UniqueFlowFiles.name, unique_flow_file_)) { +logger_->log_trace("Unique Flow files is configured to be {}", unique_flow_file_); } std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { -if (textData_ && !uniqueFlowFile_) { - data_.assign(custom_text.begin(), custom_text.end()); +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); return; -} else { - logger_->log_warn("Custom Text property is set, but not used!"); } +logger_->log_warn("Custom Text property is set, but not used!"); } - if (!uniqueFlowFile_) { -data_.resize(gsl::narrow(fileSize_)); -generateData(data_, textData_); + if (!unique_flow_file_) { +non_unique_data_.resize(gsl::narrow(file_size_)); +generateData(non_unique_data_, textData_); + } +} + +// If the Data Format is text and if Unique FlowFiles is false, the custom text has to be evaluated once per batch +void GenerateFlowFile::regenerateNonUniqueData(core::ProcessContext& context) { + std::string custom_text; + context.getProperty(CustomText, custom_text, nullptr); + if (!custom_text.empty()) { +if (textData_ && !unique_flow_file_) { + non_unique_data_.assign(custom_text.begin(), custom_text.end()); + return; +} +logger_->log_warn("Custom Text property is set, but not used!"); Review Comment: I think this would be simpler, and would avoid doing some unnecessary work: ```c++ if (!textData_ || unique_flow_file_) return; std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); if (!custom_text.empty()) { non_unique_data_.assign(custom_text.begin(), custom_text.end()); } ``` We would lose the warning, but we have warned in `onSchedule()` already. Also, if the expanded custom text is empty now, but it wasn't empty before, I think the flow file should be empty, so ```c++ if (!textData_ || unique_flow_file_) return; std::string custom_text; context.getProperty(CustomText, custom_text, nullptr); non_unique_data_.assign(custom_text.begin(), custom_text.end()); ``` would be better. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] MINIFICPP-2264 GenerateFlowFile: 'Custom Text' should be reevaluated … [nifi-minifi-cpp]
martinzink opened a new pull request, #1706: URL: https://github.com/apache/nifi-minifi-cpp/pull/1706 …once per batch Thank you for submitting a contribution to Apache NiFi - MiNiFi C++. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Does your PR title start with MINIFICPP- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [ ] Has your PR been rebased against the latest commit within the target branch (typically main)? - [ ] Is your initial contribution a single, squashed commit? ### For code changes: - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file? - [ ] If applicable, have you updated the NOTICE file? ### For documentation related changes: - [ ] Have you ensured that format looks appropriate for the output in which it is rendered? ### Note: Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org