[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1237: MINIFICPP-1224 Add module directory support to script execution

2022-01-21 Thread GitBox


adamdebreceni commented on a change in pull request #1237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1237#discussion_r789487612



##
File path: extensions/script/ExecuteScript.cpp
##
@@ -123,6 +128,10 @@ void ExecuteScript::onTrigger(const 
std::shared_ptr &conte
 throw std::runtime_error("No script engine available");
   }
 
+  if (module_directory_.size()) {

Review comment:
   should we use `!module_directory_.empty()`?
   
   (or better yet `std::optional`, although I see that this member 
is not a new addition)




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




[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1237: MINIFICPP-1224 Add module directory support to script execution

2022-01-21 Thread GitBox


adamdebreceni commented on a change in pull request #1237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1237#discussion_r789584630



##
File path: extensions/librdkafka/ConsumeKafka.cpp
##
@@ -211,7 +211,7 @@ void ConsumeKafka::onSchedule(core::ProcessContext* 
context, core::ProcessSessio
   context->getProperty(DuplicateHeaderHandling.getName(), 
duplicate_header_handling_);
 
   headers_to_add_as_attributes_ = 
utils::listFromCommaSeparatedProperty(*context, 
HeadersToAddAsAttributes.getName());
-  max_poll_records_ = 
gsl::narrow(utils::getOptionalUintProperty(*context, 
MaxPollRecords.getName()).value_or(DEFAULT_MAX_POLL_RECORDS));
+  max_poll_records_ = 
gsl::narrow(utils::getOptionalProperty(*context, 
MaxPollRecords.getName()).value_or(DEFAULT_MAX_POLL_RECORDS));

Review comment:
   `ProcessContext::getProperty(const Property&)` already returns an 
`std::optional`, does `getOptionalProperty` behave differently?




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