subject:"\[GitHub\] \[nifi\-minifi\-cpp\] szaszm commented on a change in pull request #710\: MINIFICPP \- 1110, 1111 \- PublishKafka, OPC processors should config and"

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

2020-02-07 Thread GitBox

szaszm commented on a change in pull request #710: MINIFICPP - 1110, - 
PublishKafka, OPC processors should config and
URL: https://github.com/apache/nifi-minifi-cpp/pull/710#discussion_r376419716
 
 

 ##
 File path: libminifi/include/core/state/Value.h
 ##
 @@ -115,6 +125,64 @@ class Value {
   std::type_index type_id;
 };
 
+class UInt32Value : public Value {
+ public:
+  explicit UInt32Value(uint32_t value)
+  : Value(std::to_string(value)),
+value(value) {
+setTypeId();
+  }
+
+  explicit UInt32Value(const std::string )
+  : Value(strvalue),
+value(std::stoul(strvalue)) {
+/**
+ * This is a fundamental change in that we would be changing where this 
error occurs.
+ * We should be prudent about breaking backwards compatibility, but since 
Uint32Value
+ * is only created with a validator and type, we **should** be okay.
+ */
+const auto negative = strvalue.find_first_of('-') != std::string::npos;
+ if (negative){
+   throw std::out_of_range("negative value detected");
+ }
+setTypeId();
+  }
+
+  uint32_t getValue() const {
+return value;
+  }
+ protected:
+
+  virtual bool getValue(uint32_t ) {
+ref = value;
+return true;
+  }
+
+  virtual bool getValue(int ) {
+if (value <= (std::numeric_limits::max)()) {
 
 Review comment:
   My bad, I confused `value` with `ref`


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

2020-02-06 Thread GitBox

szaszm commented on a change in pull request #710: MINIFICPP - 1110, - 
PublishKafka, OPC processors should config and
URL: https://github.com/apache/nifi-minifi-cpp/pull/710#discussion_r375847455
 
 

 ##
 File path: libminifi/include/core/state/Value.h
 ##
 @@ -188,6 +264,18 @@ class BoolValue : public Value {
 }
   }
 
+  virtual bool getValue(uint32_t ) {
 
 Review comment:
   Isn't this function meant to validate and return `value` instead of just 
validating and self-assigning `ref`?
   If I'm right:
   ```
   if (value != 0 && value != 1) { return false; }
   ref = (value != 0);
   return true;
   ```
   If the protocol allows it, I'd even go as far as skipping the validation and 
interpreting everything non-zero as true.
   
   Also: "A member function template shall not be virtual."
   source: C++ standard draft, 13.7.2 [temp.mem] 3), 
http://open-std.org/JTC1/SC22/WG21/docs/papers/2019/n4835.pdf 


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

2020-02-06 Thread GitBox

szaszm commented on a change in pull request #710: MINIFICPP - 1110, - 
PublishKafka, OPC processors should config and
URL: https://github.com/apache/nifi-minifi-cpp/pull/710#discussion_r375844201
 
 

 ##
 File path: libminifi/include/core/state/Value.h
 ##
 @@ -115,6 +125,64 @@ class Value {
   std::type_index type_id;
 };
 
+class UInt32Value : public Value {
+ public:
+  explicit UInt32Value(uint32_t value)
+  : Value(std::to_string(value)),
+value(value) {
+setTypeId();
+  }
+
+  explicit UInt32Value(const std::string )
+  : Value(strvalue),
+value(std::stoul(strvalue)) {
+/**
+ * This is a fundamental change in that we would be changing where this 
error occurs.
+ * We should be prudent about breaking backwards compatibility, but since 
Uint32Value
+ * is only created with a validator and type, we **should** be okay.
+ */
+const auto negative = strvalue.find_first_of('-') != std::string::npos;
+ if (negative){
+   throw std::out_of_range("negative value detected");
+ }
+setTypeId();
+  }
+
+  uint32_t getValue() const {
+return value;
+  }
+ protected:
+
+  virtual bool getValue(uint32_t ) {
+ref = value;
+return true;
+  }
+
+  virtual bool getValue(int ) {
+if (value <= (std::numeric_limits::max)()) {
 
 Review comment:
   This condition is always `true`. Signed integer overflow results in 
undefined behavior.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

2020-02-06 Thread GitBox

szaszm commented on a change in pull request #710: MINIFICPP - 1110, - 
PublishKafka, OPC processors should config and
URL: https://github.com/apache/nifi-minifi-cpp/pull/710#discussion_r375878858
 
 

 ##
 File path: libminifi/include/core/state/Value.h
 ##
 @@ -87,6 +88,15 @@ class Value {
 type_id = std::type_index(typeid(T));
   }
 
+  virtual bool getValue(uint32_t ) {
+const auto negative = string_value.find_first_of('-') != std::string::npos;
 
 Review comment:
   @arpadboda: The name helps readability and since it's `const` it doesn't 
increase complexity. (by potentially introducing extra states)


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

2020-02-06 Thread GitBox

szaszm commented on a change in pull request #710: MINIFICPP - 1110, - 
PublishKafka, OPC processors should config and
URL: https://github.com/apache/nifi-minifi-cpp/pull/710#discussion_r375870980
 
 

 ##
 File path: libminifi/test/integration/OnScheduleErrorHandlingTests.cpp
 ##
 @@ -29,32 +27,22 @@ class OnScheduleErrorHandlingTests : public 
IntegrationBase {
  public:
   virtual void runAssertions() {
 std::string logs = LogTestController::getInstance().log_output.str();
-size_t pos = 0;
-size_t last_pos = 0;
-unsigned int occurances = 0;
-do {
-  pos = 
logs.find(minifi::processors::KamikazeProcessor::OnScheduleExceptionStr, pos);
-  if (pos != std::string::npos) {
-last_pos = pos;
-pos = 
logs.find(minifi::processors::KamikazeProcessor::OnUnScheduleLogStr, pos);
-if (pos != std::string::npos) {
-  last_pos = pos;
-  occurances++;
-}
-  }
-} while (pos != std::string::npos);
 
-assert(occurances > 1);  // Verify retry of onSchedule and onUnSchedule 
calls
+auto result = countPatInStr(logs, 
minifi::processors::KamikazeProcessor::OnScheduleExceptionStr);
+size_t last_pos = result.first;
+unsigned int occurances = result.second;
 
 Review comment:
   typo: it's spelled occurence


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

2020-02-06 Thread GitBox

szaszm commented on a change in pull request #710: MINIFICPP - 1110, - 
PublishKafka, OPC processors should config and
URL: https://github.com/apache/nifi-minifi-cpp/pull/710#discussion_r375869797
 
 

 ##
 File path: libminifi/test/integration/IntegrationBase.h
 ##
 @@ -42,6 +42,17 @@ class IntegrationBase {
 configureSecurity();
   }
 
+  // Return the last position and number of occurrences.
+  std::pair countPatInStr(const std::string , const 
std::string ) {
+size_t last_pos = 0;
+unsigned int occurrences = 0;
 
 Review comment:
   Indeed, it should be consistent. This is minor, but I'll write it down 
anyway.
   I don't think the count type should be `uint32_t`, since there's nothing 
that mandates 32 bits of storage for our integer.
   The position is fine as `size_t`.
   
   Some candidates:
   - `int`: plain and simple, we return an integer
   - `unsigned int`: as above, but a count can not be smaller than zero and we 
can signal this with the type
   - `size_t`: The max. possible count of matches, in the case of a theoretical 
string spanning the full addressable memory and a search string matching on 
every character.
   - `ptrdiff_t`: The max. possible count of matches is the size of the string, 
that is the difference between the last and first element pointers. Note: 
signed integer
   - `typename iterator_traits::difference_type`: Same 
rationale as for `ptrdiff_t`, but for last and first iterators. This is what 
`std::count` returns.
   
   My suggestion would be a simple `int`, `unsigned int` or a correct `typename 
iterator_traits::difference_type`, in the order of my 
preference.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

2020-01-23 Thread GitBox

szaszm commented on a change in pull request #710: MINIFICPP - 1110, - 
PublishKafka, OPC processors should config and
URL: https://github.com/apache/nifi-minifi-cpp/pull/710#discussion_r370243546
 
 

 ##
 File path: libminifi/src/core/PropertyValidation.cpp
 ##
 @@ -27,6 +27,7 @@ std::shared_ptr StandardValidators::VALID 
= std::make_shared<
 StandardValidators::StandardValidators() {
   INVALID = std::make_shared(false, "INVALID");
   INTEGER_VALIDATOR = std::make_shared("INTEGER_VALIDATOR");
+  UNSIGNED_INT_VALIDATOR = 
std::make_shared("UNSIGNED_INT_VALIDATOR");
 
 Review comment:
   Java `int` is 32 bit signed, but the validator filters all the negatives so 
it's effectively 31 bit.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

2020-01-22 Thread GitBox

szaszm commented on a change in pull request #710: MINIFICPP - 1110, - 
PublishKafka, OPC processors should config and
URL: https://github.com/apache/nifi-minifi-cpp/pull/710#discussion_r369660940
 
 

 ##
 File path: extensions/librdkafka/PublishKafka.cpp
 ##
 @@ -187,81 +239,77 @@ bool PublishKafka::configureNewConnection(const 
std::shared_ptr
   auto key = conn->getKey();
 
   if (key->brokers_.empty()) {
-logger_->log_error("There are no brokers");
-return false;
+throw Exception(PROCESS_SCHEDULE_EXCEPTION, "There are no brokers");
   }
   result = rd_kafka_conf_set(conf_, "bootstrap.servers", 
key->brokers_.c_str(), errstr.data(), errstr.size());
   logger_->log_debug("PublishKafka: bootstrap.servers [%s]", key->brokers_);
   if (result != RD_KAFKA_CONF_OK) {
-logger_->log_error("PublishKafka: configure error result [%s]", 
errstr.data());
-return false;
+auto error_msg = utils::StringUtils::join_pack("PublishKafka: configure 
error result [%s]", errstr.data());
 
 Review comment:
   Maybe we could use the formatting library used by spdlog to create these 
strings? I've mentioned directly depending on the formatting library in 
[MINIFICPP-1121](https://issues.apache.org/jira/projects/MINIFICPP/issues/MINIFICPP-1121).


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #710: MINIFICPP - 1110, 1111 - PublishKafka, OPC processors should config and

8 matches

Site Navigation

Mail list logo

Footer information