[GitHub] nifi-minifi-cpp pull request #99: MINIFI-270: Move Base RPG capabilities bac...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/99#discussion_r117081374
  
--- Diff: LICENSE ---
@@ -401,6 +401,33 @@ ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 The views and conclusions contained in the software and documentation are 
those of the authors and
 should not be interpreted as representing official policies, either 
expressed or implied, of Dmitry Vyukov.
 
+
+This product bundles 'concurrentqueue' which is available a Boost Software 
License.
--- End diff --

I'll add a link to that on the wiki entry from other PR. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #99: MINIFI-270: Move Base RPG capabilities bac...

2017-05-17 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/99

MINIFI-270: Move Base RPG capabilities back into RemoteProcessGroupPort

This allows us to extend capabilities in RPG into other classes. Since
the same basic functions are the same we can bring the code up from
Processor into RemoteProcessGroupPort.h instead of a separate class.

Updated LICENSE and added concurrentqueue to locking for the object pool.

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-270

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/99.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #99


commit 3f9825e48bf340cb9694068cd5873afa28c3ed65
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-05-17T12:18:26Z

MINIFI-270: Move Base RPG capabilities back into RemoteProcessGroupPort

This allows us to extend capabilities in RPG into other classes. Since
the same basic functions are the same we can bring the code up from
Processor into RemoteProcessGroupPort.h instead of a separate class.

Updated LICENSE and added concurrentqueue to locking for the object pool.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #98: MINIFI-37: Create a volatile repository an...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/98#discussion_r117061878
  
--- Diff: libminifi/src/core/RepositoryFactory.cpp ---
@@ -48,9 +50,15 @@ std::shared_ptr createRepository(
   try {
 std::shared_ptr return_obj = nullptr;
 if (class_name_lc == "flowfilerepository") {
+  std::cout << "creating flow" << std::endl;
--- End diff --

sorry I added that to be captured by a test, but I guess I forgot the 
outputstreamappender can redirect to stdout. I'll fix. Thanks for reminding me. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #98: MINIFI-37: Create a volatile repository an...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/98#discussion_r117061976
  
--- Diff: libminifi/src/core/RepositoryFactory.cpp ---
@@ -48,9 +50,15 @@ std::shared_ptr createRepository(
   try {
 std::shared_ptr return_obj = nullptr;
 if (class_name_lc == "flowfilerepository") {
+  std::cout << "creating flow" << std::endl;
   return_obj = instantiate();
 } else if (class_name_lc == "provenancerepository") {
   return_obj = instantiate();
+} else if (class_name_lc == "volatilerepository") {
+  return_obj = instantiate();
+} else if (class_name_lc == "nooprepository") {
+  std::cout << "creating noop" << std::endl;
--- End diff --

it's needed for a test, but the test should be using the 
outputstreamappender. will fix. thanks. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #99: MINIFI-270: Move Base RPG capabilities bac...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/99#discussion_r117079885
  
--- Diff: LICENSE ---
@@ -401,6 +401,33 @@ ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 The views and conclusions contained in the software and documentation are 
those of the authors and
 should not be interpreted as representing official policies, either 
expressed or implied, of Dmitry Vyukov.
 
+
+This product bundles 'concurrentqueue' which is available a Boost Software 
License.
--- End diff --

https://github.com/cameron314/concurrentqueue/blob/master/LICENSE.md


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #99: MINIFI-270: Move Base RPG capabilities bac...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/99#discussion_r117080960
  
--- Diff: libminifi/src/core/Processor.cpp ---
@@ -194,52 +193,10 @@ void 
Processor::removeConnection(std::shared_ptr conn) {
   }
 }
 
-std::shared_ptr 
Processor::obtainSite2SiteProtocol(
-const std::shared_ptr _factory, std::string 
host, uint16_t sport, uuid_t portId) {
-  std::lock_guard < std::mutex > lock(mutex_);
+/*
--- End diff --

but thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #99: MINIFI-270: Move Base RPG capabilities bac...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/99#discussion_r117079731
  
--- Diff: libminifi/src/core/Processor.cpp ---
@@ -194,52 +193,10 @@ void 
Processor::removeConnection(std::shared_ptr conn) {
   }
 }
 
-std::shared_ptr 
Processor::obtainSite2SiteProtocol(
-const std::shared_ptr _factory, std::string 
host, uint16_t sport, uuid_t portId) {
-  std::lock_guard < std::mutex > lock(mutex_);
+/*
--- End diff --

Sorry I was still self reviewing this. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi issue #1805: NIFI-3905: When a Provenance Query is submitted to WriteAh...

2017-05-16 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi/pull/1805
  
Looks good but would be nice if there were tests for this if that is 
possible to replicate statically. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #97: MINIFI-289: Update test folder to apply li...

2017-05-16 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/97

MINIFI-289: Update test folder to apply linter and set max characters…

… to 200

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-289

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/97.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #97


commit 5874c8b3f1fa646fcf1d30795410340962354290
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-05-16T17:58:28Z

MINIFI-289: Update test folder to apply linter and set max characters to 200




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #83: MINIFI-226: Add controller services capabi...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/83#discussion_r116983866
  
--- Diff: libminifi/test/Test.cpp ---
@@ -1,6 +1,4 @@
 /**
- * @file MiNiFiMain.cpp 
--- End diff --

Ah yeah this one is per another PR I have. I'll remove it here since the 
other PR would need to be rebased, anyway. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #83: MINIFI-226: Add controller services capabi...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/83#discussion_r116968301
  
--- Diff: libminifi/test/unit/ClassLoaderTests.cpp ---
@@ -0,0 +1,48 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "../../include/core/ClassLoader.h"
+#include "../TestBase.h"
+#include "io/ClientSocket.h"
+#include "core/Processor.h"
+#include "core/ClassLoader.h"
+#include "processors/AppendHostInfo.h"
+#include "core/logging/LogAppenders.h"
+
+using namespace org::apache::nifi::minifi::io;
+TEST_CASE("TestLoader", "[TestLoader]") {
+  
+REQUIRE ( nullptr != 
core::ClassLoader::getDefaultClassLoader().instantiate("AppendHostInfo","hosty"));
+REQUIRE ( nullptr != 
core::ClassLoader::getDefaultClassLoader().instantiate("ListenHTTP","hosty2"));
+REQUIRE ( nullptr == 
core::ClassLoader::getDefaultClassLoader().instantiate("Don'tExist","hosty3"));
+REQUIRE ( nullptr == 
core::ClassLoader::getDefaultClassLoader().instantiate("","EmptyEmpty"));
+/*
--- End diff --

doh!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #83: MINIFI-226: Add controller services capabi...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/83#discussion_r116968249
  
--- Diff: libminifi/test/resources/TestControllerServices.yml ---
@@ -0,0 +1,62 @@
+Flow Controller:
--- End diff --

Yah. that was a last minute copy when I rebased. I'll get rid of that and 
add a .gitignore. thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #83: MINIFI-226: Add controller services capabi...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/83#discussion_r116968270
  
--- Diff: libminifi/test/resources/cn.crt.pem ---
@@ -0,0 +1,25 @@
+Bag Attributes
--- End diff --

yes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #83: MINIFI-226: Add controller services capabi...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/83#discussion_r116968276
  
--- Diff: libminifi/test/resources/nifi-cert.pem ---
@@ -0,0 +1,20 @@
+-BEGIN CERTIFICATE-
--- End diff --

yes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #83: MINIFI-226: Add controller services capabi...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/83#discussion_r116968835
  
--- Diff: libminifi/include/processors/ListenHTTP.h ---
@@ -115,6 +114,9 @@ class ListenHTTP : public core::Processor {
   std::unique_ptr _handler;
 };
 
+
+REGISTER_RESOURCE(ListenHTTP);
--- End diff --

yes I thought about this morning that the readme needs some love. I'll do 
that. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #83: MINIFI-226: Add controller services capabi...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/83#discussion_r116968718
  
--- Diff: libminifi/test/resources/cn.ckey.pem ---
@@ -0,0 +1,31 @@
+Bag Attributes
--- End diff --

yes. i just used the toolkit to generate them. Should I provide that info 
somewhere in the readme?
Owner: CN=test, OU=NIFI
Issuer: CN=localhost, OU=NIFI
Serial number: 15a67536961
Valid from: Wed Feb 22 14:36:44 EST 2017 until: Sat Feb 22 14:36:44 EST 2020


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #98: MINIFI-37: Create a volatile repository an...

2017-05-16 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/98

MINIFI-37: Create a volatile repository and config items for

NoOp and Volatile repository configuration

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-37

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/98.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #98


commit a2374ca1a7d636e3b9c5abb8b6d97753b5ddb311
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-05-12T19:22:06Z

MINIFI-37: Create a volatile repository and config items for
NoOp and Volatile repository configuration




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #96: MINIFI-304: Update testing strategy

2017-05-17 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/96
  
@apiri I would prefer to wait on #83 . 




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #83: MINIFI-226: Add controller services capabi...

2017-05-17 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/83#discussion_r117021448
  
--- Diff: libminifi/src/SchedulingAgent.cpp ---
@@ -39,6 +40,36 @@ bool 
SchedulingAgent::hasWorkToDo(std::shared_ptr processor) {
 return false;
 }
 
+void SchedulingAgent::enableControllerService(
+std::shared_ptr ) 
{
+
+  logger_->log_trace("Enabling CSN in SchedulingAgent %s",
+ serviceNode->getName());
+  // reference the enable function from serviceNode
+  std::function<bool()> f_ex = [serviceNode] {
+return serviceNode->enable();
+  };
+  // create a functor that will be submitted to the thread pool.
+  utils::Worker functor(f_ex);
+  // move the functor into the thread pool. While a future is returned
+  // we aren't terribly concerned with the result.
+  component_lifecycle_thread_pool_.execute(std::move(functor));
+}
+
+void SchedulingAgent::disableControllerService(
--- End diff --

The FlowController is an implementation of a controller service provider. 
It will start the controller services and disable them at shutdown to allow for 
cleanup. There's still some questions in my mind if that is adequate (i.e. 
we're using a timer in some spots ), but that is a separate issue and  captured 
in a ticket.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #110: MINIFI-249: Update prov repo to better ab...

2017-06-20 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/110#discussion_r123120308
  
--- Diff: libminifi/include/core/repository/AtomicRepoEntries.h ---
@@ -0,0 +1,501 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ref_count_hip.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef LIBMINIFI_INCLUDE_CORE_REPOSITORY_ATOMICREPOENTRIES_H_
+#define LIBMINIFI_INCLUDE_CORE_REPOSITORY_ATOMICREPOENTRIES_H_
+
+#include  
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static uint16_t accounting_size = sizeof(std::vector) + 
sizeof(std::string) + sizeof(size_t);
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+namespace repository {
+
+/**
+ * Purpose: Repo value represents an item that will support a move 
operation within an AtomicEntry
+ *
+ * Justification: Since AtomicEntry is a static entry that does not move 
or change, the underlying
+ * RepoValue can be changed to support atomic operations.
+ */
+template
+class RepoValue {
+ public:
+
+  explicit RepoValue() {
+  }
+
+  /**
+   * Constructor that populates the item allowing for a custom key 
comparator.
+   * @param key key for this repo value.
+   * @param ptr buffer
+   * @param size size buffer
+   * @param comparator custom comparator.
+   */
+  explicit RepoValue(T key, const uint8_t *ptr, size_t size, 
std::function<bool(T, T)> comparator = nullptr)
+  : key_(key),
+comparator_(comparator) {
+if (nullptr == ptr) {
+  size = 0;
+}
+buffer_.resize(size);
+if (size > 0) {
+  std::memcpy(buffer_.data(), ptr, size);
+}
+  }
+
+  /**
+   * RepoValue that moves the other object into this.
+   */
+  explicit RepoValue(RepoValue &)
+noexcept  : key_(std::move(other.key_)),
+  buffer_(std::move(other.buffer_)),
+  comparator_(std::move(other.comparator_)) {
+  }
+
+  ~RepoValue()
+  {
+  }
+
+  T () {
+return key_;
+  }
+
+  /**
+   * Sets the key, relacing the custom comparator if needed.
+   */
+  void setKey(const T key, std::function<bool(T,T)> comparator = 
nullptr) {
+key_ = key;
+comparator_ = comparator;
+  }
+
+  /**
+   * Determines if the key is the same using the custom comparator
+   * @param other object to compare against
+   * @return result of the comparison
+   */
+  inline bool isEqual(RepoValue *other)
+  {
+return comparator_ == nullptr ? key_ == other->key_ : 
comparator_(key_,other->key_);
+  }
+
+  /**
+   * Determines if the key is the same using the custom comparator
+   * @param other object to compare against
+   * @return result of the comparison
+   */
+  inline bool isKey(T other)
+  {
+return comparator_ == nullptr ? key_ == other : 
comparator_(key_,other);
+  }
+
+  /**
+   * Clears the buffer.
+   */
+  void clearBuffer() {
+buffer_.resize(0);
+buffer_.clear();
+  }
+
+  /**
+   * Return the size of the memory within the key
+   * buffer, the size of timestamp, and the general
+   * system word size
+   */
+  uint64_t size() {
+return buffer_.size();
+  }
+
+  size_t getBufferSize() {
+return buffer_.size();
+  }
+
+  const uint8_t *getBuffer()
+  {
+return buffer_.data();
+  }
+
+  /**
+   * Places the contents of buffer into str
+   * @param strnig into which we are placing the memory contained in 
buffer.
+

[GitHub] nifi-minifi-cpp pull request #110: MINIFI-249: Update prov repo to better ab...

2017-06-20 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/110#discussion_r123119025
  
--- Diff: libminifi/include/core/repository/VolatileRepository.h ---
@@ -331,22 +132,240 @@ class VolatileRepository : public core::Repository, 
public std::enable_shared_fr
 else
   return false;
   }
-  /**
-   * Purges the volatile repository.
-   */
-  void purge();
 
- private:
   std::map<std::string, std::shared_ptr> connectionMap;
-
-  std::atomic current_size_;
+  // current size of the volatile repo.
+  std::atomic current_size_;
+  // current index.
   std::atomic current_index_;
-  std::vector<AtomicEntry*> value_vector_;
+  // value vector.
--- End diff --

This is a base class. the super class MAY use a map in certain use cases. 
see https://github.com/apache/nifi-minifi-cpp/pull/113 for when the minimized 
locking is used. the vector here has the ability to be completely lock free for 
certain use cases. the computational complexity is pretty low versus the 
locking required. value_vector_ exists for the repositories that want complete 
lock free operations via CAS


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #110: MINIFI-249: Update prov repo to better abstract ...

2017-06-20 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/110
  
@benqiu2016 
  1) Yes, I have run for two days. Have also run valgrind to show no memory 
leaks
  2) Yes, and in this case I would encourage the users to use the file 
system based repositories. The benefit to using the volatile repo is that we 
don't need to use the file system API ( and potentially context switches) . It 
may be advantageous to create a file stream that uses mmap instead of open. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #113: MINIFI-337: Update configuration readme f...

2017-06-19 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/113

MINIFI-337: Update configuration readme for Repositories

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-337

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/113.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #113


commit 450383ae1bf27d4677d92b4e8006142cf79355c2
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-06-19T13:33:17Z

MINIFI-337: Update configuration readme for Repositories




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #113: MINIFI-337: Update configuration readme for Repo...

2017-06-20 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/113
  
@apiri This PR follows #110 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #110: MINIFI-249: Update prov repo to better ab...

2017-06-20 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/110#discussion_r123113442
  
--- Diff: libminifi/include/core/FlowFile.h ---
@@ -224,6 +224,10 @@ class FlowFile {
 
   void setStoredToRepository(bool storedInRepository) {
 stored = storedInRepository;
+if (!stored && nullptr != claim_)
+{
+  claim_->decreaseFlowFileRecordOwnedCount();
--- End diff --

Initially I added this because I used storage in the repo as part of the 
count, but I removed this concept hence this isn't necessary. I will remove it, 
thanks. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-22 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123504131
  
--- Diff: libminifi/src/RemoteProcessorGroupPort.cpp ---
@@ -76,27 +89,20 @@ void 
RemoteProcessorGroupPort::returnProtocol(std::unique_ptr properties;
-  properties.insert(hostName);
-  properties.insert(port);
   properties.insert(portUUID);
   setSupportedProperties(properties);
 // Set the supported relationships
   std::set relationships;
   relationships.insert(relation);
   setSupportedRelationships(relationships);
+  curl_global_init(CURL_GLOBAL_DEFAULT);
--- End diff --

the function is not thread safe. initialization isn't guaranteed to be 
thread safe either since other components could initialize concurrently. It's 
probably fine for now but seems like this should be done in a thread safe way. 
https://curl.haxx.se/libcurl/c/curl_global_init.html


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-22 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123503771
  
--- Diff: libminifi/src/RemoteProcessorGroupPort.cpp ---
@@ -54,19 +57,29 @@ bool create = true) {
   std::unique_ptr nextProtocol = nullptr;
   if (!available_protocols_.try_dequeue(nextProtocol)) {
 if (create) {
+  refreshPeerList();
   // create
-  nextProtocol = std::unique_ptr(new 
Site2SiteClientProtocol(nullptr));
-  nextProtocol->setPortId(protocol_uuid_);
-  std::unique_ptr str = 
std::unique_ptr(stream_factory_->createSocket(host_,
 port_));
-  std::unique_ptr peer_ = 
std::unique_ptr(new Site2SitePeer(std::move(str), host_, port_));
-  nextProtocol->setPeer(std::move(peer_));
+  for (auto peer : site2site_peer_status_list_) {
--- End diff --

Could you not create one on demand and simply return it versus enqueueing 
it and dequeuing it? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-22 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123501252
  
--- Diff: libminifi/src/Site2SiteClientProtocol.cpp ---
@@ -344,7 +427,8 @@ int 
Site2SiteClientProtocol::readRequestType(RequestType ) {
   return -1;
 }
 
-int Site2SiteClientProtocol::readRespond(RespondCode , std::string 
) {
+int Site2SiteClientProtocol::readRespond(RespondCode ,
--- End diff --

Why the change in the function prototype? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-22 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123503614
  
--- Diff: libminifi/src/RemoteProcessorGroupPort.cpp ---
@@ -54,19 +57,29 @@ bool create = true) {
   std::unique_ptr nextProtocol = nullptr;
   if (!available_protocols_.try_dequeue(nextProtocol)) {
 if (create) {
+  refreshPeerList();
   // create
-  nextProtocol = std::unique_ptr(new 
Site2SiteClientProtocol(nullptr));
-  nextProtocol->setPortId(protocol_uuid_);
-  std::unique_ptr str = 
std::unique_ptr(stream_factory_->createSocket(host_,
 port_));
-  std::unique_ptr peer_ = 
std::unique_ptr(new Site2SitePeer(std::move(str), host_, port_));
-  nextProtocol->setPeer(std::move(peer_));
+  for (auto peer : site2site_peer_status_list_) {
+std::unique_ptr protocol = nullptr;
+protocol = std::unique_ptr < Site2SiteClientProtocol
+> (new Site2SiteClientProtocol(nullptr));
+protocol->setPortId(protocol_uuid_);
+std::unique_ptr str =
+std::unique_ptr < org::apache::nifi::minifi::io::DataStream
+> (stream_factory_->createSocket(peer.host_, peer.port_));
+std::unique_ptr peer_ = std::unique_ptr < 
Site2SitePeer
+> (new Site2SitePeer(std::move(str), peer.host_, peer.port_));
+protocol->setPeer(std::move(peer_));
+returnProtocol(std::move(protocol));
+  }
 }
+available_protocols_.try_dequeue(nextProtocol);
   }
   return std::move(nextProtocol);
 }
 
 void 
RemoteProcessorGroupPort::returnProtocol(std::unique_ptr
 return_protocol) {
-  if (available_protocols_.size_approx() >= max_concurrent_tasks_) {
+  if (available_protocols_.size_approx() >= (max_concurrent_tasks_ * 
site2site_peer_status_list_.size())) {
--- End diff --

Why should this change? 
With 10 concurrent tasks ( threads ) and 10 peers, you need 100 queued 
elements? Shouldn't you only need ten 
 ( +/- a few ) since you only have ten threads to pull objects? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-22 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123502816
  
--- Diff: libminifi/src/core/yaml/YamlConfiguration.cpp ---
@@ -314,10 +314,8 @@ void 
YamlConfiguration::parseProvenanceReportingYaml(YAML::Node *reportNode, cor
   auto schedulingStrategyStr = node["scheduling 
strategy"].as();
   checkRequiredField(, "scheduling period", 
CONFIG_YAML_PROVENANCE_REPORT_KEY);
   auto schedulingPeriodStr = node["scheduling period"].as();
-  checkRequiredField(, "host", CONFIG_YAML_PROVENANCE_REPORT_KEY);
-  auto hostStr = node["host"].as();
-  checkRequiredField(, "port", CONFIG_YAML_PROVENANCE_REPORT_KEY);
-  auto portStr = node["port"].as();
+  checkRequiredField(, "url", CONFIG_YAML_PROVENANCE_REPORT_KEY);
--- End diff --

We should maintain prior functionality. Though we're not GA, we should 
avoid jarring consumers who may be using a prior config. We should able to 
support both conditionally. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-22 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123500630
  
--- Diff: libminifi/src/Site2SiteClientProtocol.cpp ---
@@ -38,7 +39,8 @@ namespace minifi {
 
 bool Site2SiteClientProtocol::establish() {
   if (_peerState != IDLE) {
-logger_->log_error("Site2Site peer state is not idle while try to 
establish");
+logger_->log_error(
--- End diff --

Why the change in log statements? Our line length is 200. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-22 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123500565
  
--- Diff: libminifi/src/RemoteProcessorGroupPort.cpp ---
@@ -150,6 +147,87 @@ void 
RemoteProcessorGroupPort::onTrigger(core::ProcessContext *context, core::Pr
   return;
 }
 
+void RemoteProcessorGroupPort::refreshRemoteSite2SiteInfo() {
+  if (this->host_.empty() || this->port_ == -1 || this->protocol_.empty())
+  return;
+
+  std::string fullUrl = this->protocol_ + this->host_ + ":" + 
std::to_string(this->port_) + "/nifi-api/controller/";
+
+  this->site2site_port_ = -1;
+  CURL *http_session = curl_easy_init();
+
+  curl_easy_setopt(http_session, CURLOPT_URL, fullUrl.c_str());
+
+  utils::HTTPRequestResponse content;
+  curl_easy_setopt(http_session, CURLOPT_WRITEFUNCTION,
+  ::HTTPRequestResponse::recieve_write);
+
+  curl_easy_setopt(http_session, CURLOPT_WRITEDATA,
+  static_cast<void*>());
+
+  CURLcode res = curl_easy_perform(http_session);
+
+  if (res == CURLE_OK) {
+std::string response_body(content.data.begin(), content.data.end());
+int64_t http_code = 0;
+curl_easy_getinfo(http_session, CURLINFO_RESPONSE_CODE, _code);
+char *content_type;
+/* ask for the content-type */
+curl_easy_getinfo(http_session, CURLINFO_CONTENT_TYPE, _type);
+
+bool isSuccess = ((int32_t) (http_code / 100)) == 2
+&& res != CURLE_ABORTED_BY_CALLBACK;
+bool body_empty = IsNullOrEmpty(content.data);
+
+if (isSuccess && !body_empty) {
+  std::string controller = std::move(response_body);
+  logger_->log_debug("controller config %s", controller.c_str());
+  Json::Value value;
+  Json::Reader reader;
+  bool parsingSuccessful = reader.parse(controller, value);
+  if (parsingSuccessful && !value.empty()) {
+Json::Value controllerValue = value["controller"];
+if (!controllerValue.empty()) {
+  Json::Value port = controllerValue["remoteSiteListeningPort"];
+  if (!port.empty())
+this->site2site_port_ = port.asInt();
+  Json::Value secure = controllerValue["siteToSiteSecure"];
+  if (!secure.empty())
+this->site2site_secure_ = secure.asBool();
+}
+logger_->log_info("process group remote site2site port %d, is 
secure %d", site2site_port_, site2site_secure_);
+  }
+} else {
+  logger_->log_error("Cannot output body to content for 
ProcessGroup::refreshRemoteSite2SiteInfo");
+}
+  } else {
+logger_->log_error(
+"ProcessGroup::refreshRemoteSite2SiteInfo -- curl_easy_perform() 
failed %s\n",
+curl_easy_strerror(res));
+  }
+  curl_easy_cleanup(http_session);
+}
+
+void RemoteProcessorGroupPort::refreshPeerList() {
+  refreshRemoteSite2SiteInfo();
+  if (site2site_port_ == -1)
+return;
+
+  this->site2site_peer_status_list_.clear();
+
+  std::unique_ptr < Site2SiteClientProtocol> protocol;
+  protocol = std::unique_ptr < Site2SiteClientProtocol
+  > (new Site2SiteClientProtocol(nullptr));
--- End diff --

why the need to create a new object each refresh? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-22 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123500940
  
--- Diff: libminifi/src/Site2SiteClientProtocol.cpp ---
@@ -278,26 +301,28 @@ bool Site2SiteClientProtocol::handShake() {
   }
 
   switch (code) {
-case PROPERTIES_OK:
-  logger_->log_info("Site2Site HandShake Completed");
-  _peerState = HANDSHAKED;
-  return true;
-case PORT_NOT_IN_VALID_STATE:
-case UNKNOWN_PORT:
-case PORTS_DESTINATION_FULL:
-  logger_->log_error("Site2Site HandShake Failed because destination 
port is either invalid or full");
-  ret = -1;
-  /*
-   peer_->yield();
-   tearDown(); */
-  return false;
-default:
-  logger_->log_info("HandShake Failed because of unknown respond code 
%d", code);
-  ret = -1;
-  /*
-   peer_->yield();
-   tearDown(); */
-  return false;
+  case PROPERTIES_OK:
+logger_->log_info("Site2Site HandShake Completed");
+_peerState = HANDSHAKED;
+return true;
+  case PORT_NOT_IN_VALID_STATE:
+  case UNKNOWN_PORT:
+  case PORTS_DESTINATION_FULL:
+logger_->log_error(
+"Site2Site HandShake Failed because destination port is either 
invalid or full");
+ret = -1;
+/*
--- End diff --

is the commented code needed here? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-22 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123501069
  
--- Diff: libminifi/src/Site2SiteClientProtocol.cpp ---
@@ -319,6 +344,64 @@ void Site2SiteClientProtocol::tearDown() {
   _peerState = IDLE;
 }
 
+bool 
Site2SiteClientProtocol::getPeerList(std::vector 
) {
+  if (establish() && handShake()) {
+int status = this->writeRequestType(REQUEST_PEER_LIST);
+
+if (status <= 0) {
+  tearDown();
+  return false;
+}
+
+uint32_t number;
+status = peer_->read(number);
+
+if (status <= 0) {
+  tearDown();
+  return false;
+}
+
+for (int i = 0; i < number; i++) {
--- End diff --

Does it make sense to have a limit on number so we don't have a virtual DoS?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123768801
  
--- Diff: libminifi/src/RemoteProcessorGroupPort.cpp ---
@@ -54,19 +57,29 @@ bool create = true) {
   std::unique_ptr nextProtocol = nullptr;
   if (!available_protocols_.try_dequeue(nextProtocol)) {
 if (create) {
+  refreshPeerList();
   // create
-  nextProtocol = std::unique_ptr(new 
Site2SiteClientProtocol(nullptr));
-  nextProtocol->setPortId(protocol_uuid_);
-  std::unique_ptr str = 
std::unique_ptr(stream_factory_->createSocket(host_,
 port_));
-  std::unique_ptr peer_ = 
std::unique_ptr(new Site2SitePeer(std::move(str), host_, port_));
-  nextProtocol->setPeer(std::move(peer_));
+  for (auto peer : site2site_peer_status_list_) {
+std::unique_ptr protocol = nullptr;
+protocol = std::unique_ptr < Site2SiteClientProtocol
+> (new Site2SiteClientProtocol(nullptr));
+protocol->setPortId(protocol_uuid_);
+std::unique_ptr str =
+std::unique_ptr < org::apache::nifi::minifi::io::DataStream
+> (stream_factory_->createSocket(peer.host_, peer.port_));
+std::unique_ptr peer_ = std::unique_ptr < 
Site2SitePeer
+> (new Site2SitePeer(std::move(str), peer.host_, peer.port_));
+protocol->setPeer(std::move(peer_));
+returnProtocol(std::move(protocol));
+  }
 }
+available_protocols_.try_dequeue(nextProtocol);
   }
   return std::move(nextProtocol);
 }
 
 void 
RemoteProcessorGroupPort::returnProtocol(std::unique_ptr
 return_protocol) {
-  if (available_protocols_.size_approx() >= max_concurrent_tasks_) {
+  if (available_protocols_.size_approx() >= (max_concurrent_tasks_ * 
site2site_peer_status_list_.size())) {
--- End diff --

I think the logic works for a single thread, but what happens in the case 
where you have ten threads, returning at different times? Typical algorithms 
use multiple queues for fairness. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123769093
  
--- Diff: libminifi/src/RemoteProcessorGroupPort.cpp ---
@@ -54,19 +57,29 @@ bool create = true) {
   std::unique_ptr nextProtocol = nullptr;
   if (!available_protocols_.try_dequeue(nextProtocol)) {
 if (create) {
+  refreshPeerList();
   // create
-  nextProtocol = std::unique_ptr(new 
Site2SiteClientProtocol(nullptr));
-  nextProtocol->setPortId(protocol_uuid_);
-  std::unique_ptr str = 
std::unique_ptr(stream_factory_->createSocket(host_,
 port_));
-  std::unique_ptr peer_ = 
std::unique_ptr(new Site2SitePeer(std::move(str), host_, port_));
-  nextProtocol->setPeer(std::move(peer_));
+  for (auto peer : site2site_peer_status_list_) {
--- End diff --

What is the time to establish sockets? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #115: MINIFI-324 - More flexible uid generation

2017-06-23 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/115
  
Looks good will merge shortly. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #115: MINIFI-324 - More flexible uid generation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/115#discussion_r123644079
  
--- Diff: libminifi/include/provenance/Provenance.h ---
@@ -160,21 +161,9 @@ class ProvenanceEventRecord : protected 
org::apache::nifi::minifi::io::Serializa
   /*!
* Create a new provenance event record
*/
-  ProvenanceEventRecord(ProvenanceEventType event, std::string 
componentId, std::string componentType)
-  : 
logger_(logging::LoggerFactory::getLogger()) {
-_eventType = event;
-_componentId = componentId;
-_componentType = componentType;
-_eventTime = getTimeMillis();
-char eventIdStr[37];
-// Generate the global UUID for th event
-uuid_generate(_eventId);
-uuid_unparse_lower(_eventId, eventIdStr);
-_eventIdStr = eventIdStr;
-  }
+  ProvenanceEventRecord(ProvenanceEventType event, std::string 
componentId, std::string componentType);
--- End diff --

Thanks. I'm guilty of doing this too and it's so much cleaner to move it 
out. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #115: MINIFI-324 - More flexible uid generation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/115#discussion_r123642563
  
--- Diff: libminifi/include/utils/Id.h ---
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef LIBMINIFI_INCLUDE_UTILS_ID_H_
+#define LIBMINIFI_INCLUDE_UTILS_ID_H_
+
+#include 
+#include 
+#include 
+#include 
+
+#include "core/logging/Logger.h"
+#include "properties/Properties.h"
+
+#define UNSIGNED_CHAR_MAX 255
+#define UUID_TIME_IMPL 0
+#define UUID_RANDOM_IMPL 1
+#define UUID_DEFAULT_IMPL 2
+#define MINIFI_UID_IMPL 3
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+class IdGenerator {
+ public:
+  void generate(uuid_t output);
+  void initialize(const std::shared_ptr & properties);
+  
+  static std::shared_ptr getIdGenerator() {
+static std::shared_ptr generator = 
std::shared_ptr(new IdGenerator());
--- End diff --

why not use make_shared?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #115: MINIFI-324 - More flexible uid generation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/115#discussion_r123745177
  
--- Diff: README.md ---
@@ -326,26 +326,44 @@ Additionally, users can utilize the MiNiFi Toolkit 
Converter (version 0.0.1 - sc
 
 ### Http Configuration Listener
 
-Http Configuration Listener will pull flow file configuration from the 
remote command control server, 
+Http Configuration Listener will pull flow file configuration from the 
remote command control server,
 validate and apply the new flow configuration
 
-in minifi.properties 
+in minifi.properties
 
 nifi.configuration.listener.type=http
 nifi.configuration.listener.http.url=https://localhost:8080
-nifi.configuration.listener.pull.interval=1 sec 
+nifi.configuration.listener.pull.interval=1 sec
 nifi.configuration.listener.client.ca.certificate=./conf/nifi-cert.pem
 
 if you want to enable client certificate
 nifi.configuration.listener.need.ClientAuth=true
 nifi.configuration.listener.client.certificate=./conf/client.pem
 nifi.configuration.listener.client.private.key=./conf/client.key
 nifi.configuration.listener.client.pass.phrase=./conf/password
-  
+
+### UID generation
+
+MiNiFi need to generate many unique identifiers in the course of 
operations.  There are a few different uid implementations available that can 
be configured in minifi-uid.properties.
+
+Implementation for uid generation can be selected using the 
uid.implementation property values:
+1. time - use uuid_generate_time
+2. random - use uuid_generate_random
+3. uuid_default - use uuid_generate (will attempt to use 
uuid_generate_random and fall back to uuid_generate_time if no high quality 
randomness is available)
--- End diff --

Sorry, I think my question was poorly worded. You make clear what happens 
when the file exists, which is what I meant by "in the context of 
uid.implementation [existing]". 

My question was more about documenting what will be used if the uid 
configuration file and the options don't exist. It seemed from the log message, 
"Using uuid_generate_time implementation for uids." that we were simply relying 
on time and not random first and then time. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #114: site2site port negotiation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/114#discussion_r123748661
  
--- Diff: libminifi/src/core/yaml/YamlConfiguration.cpp ---
@@ -314,10 +314,8 @@ void 
YamlConfiguration::parseProvenanceReportingYaml(YAML::Node *reportNode, cor
   auto schedulingStrategyStr = node["scheduling 
strategy"].as();
   checkRequiredField(, "scheduling period", 
CONFIG_YAML_PROVENANCE_REPORT_KEY);
   auto schedulingPeriodStr = node["scheduling period"].as();
-  checkRequiredField(, "host", CONFIG_YAML_PROVENANCE_REPORT_KEY);
-  auto hostStr = node["host"].as();
-  checkRequiredField(, "port", CONFIG_YAML_PROVENANCE_REPORT_KEY);
-  auto portStr = node["port"].as();
+  checkRequiredField(, "url", CONFIG_YAML_PROVENANCE_REPORT_KEY);
--- End diff --

If we tagged today would people who have tried MiNiFi in cpp 0.2.0 be able 
to use the same config files in cpp 0.3.0? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #115: MINIFI-324 - More flexible uid generation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/115#discussion_r123743347
  
--- Diff: README.md ---
@@ -326,26 +326,44 @@ Additionally, users can utilize the MiNiFi Toolkit 
Converter (version 0.0.1 - sc
 
 ### Http Configuration Listener
 
-Http Configuration Listener will pull flow file configuration from the 
remote command control server, 
+Http Configuration Listener will pull flow file configuration from the 
remote command control server,
 validate and apply the new flow configuration
 
-in minifi.properties 
+in minifi.properties
 
 nifi.configuration.listener.type=http
 nifi.configuration.listener.http.url=https://localhost:8080
-nifi.configuration.listener.pull.interval=1 sec 
+nifi.configuration.listener.pull.interval=1 sec
 nifi.configuration.listener.client.ca.certificate=./conf/nifi-cert.pem
 
 if you want to enable client certificate
 nifi.configuration.listener.need.ClientAuth=true
 nifi.configuration.listener.client.certificate=./conf/client.pem
 nifi.configuration.listener.client.private.key=./conf/client.key
 nifi.configuration.listener.client.pass.phrase=./conf/password
-  
+
+### UID generation
+
+MiNiFi need to generate many unique identifiers in the course of 
operations.  There are a few different uid implementations available that can 
be configured in minifi-uid.properties.
+
+Implementation for uid generation can be selected using the 
uid.implementation property values:
+1. time - use uuid_generate_time
+2. random - use uuid_generate_random
+3. uuid_default - use uuid_generate (will attempt to use 
uuid_generate_random and fall back to uuid_generate_time if no high quality 
randomness is available)
--- End diff --

This statement confused me, but I'm guessing that it's in the context of 
uid.implementation. 
The following is logged to debug if uid.implementation isn't defined ( 
let's say there is no uid properties file ) 
"Using uuid_generate_time implementation for uids."

Does it make sense to make clear that uuid_generate_time is used as the 
fallback if the file does not exist? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #115: MINIFI-324 - More flexible uid generation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/115#discussion_r123643744
  
--- Diff: libminifi/src/provenance/Provenance.cpp ---
@@ -34,9 +36,24 @@ namespace nifi {
 namespace minifi {
 namespace provenance {
 
+std::shared_ptr ProvenanceEventRecord::id_generator_ = 
utils::IdGenerator::getIdGenerator();
+std::shared_ptr ProvenanceEventRecord::logger_ = 
logging::LoggerFactory::getLogger();
+
 const char *ProvenanceEventRecord::ProvenanceEventTypeStr[REPLAY + 1] = { 
"CREATE", "RECEIVE", "FETCH", "SEND", "DOWNLOAD", "DROP", "EXPIRE", "FORK", 
"JOIN", "CLONE", "CONTENT_MODIFIED",
 "ATTRIBUTES_MODIFIED", "ROUTE", "ADDINFO", "REPLAY" };
 

+ProvenanceEventRecord::ProvenanceEventRecord(ProvenanceEventRecord::ProvenanceEventType
 event, std::__cxx11::string componentId, std::__cxx11::string componentType) {
--- End diff --

was the use of std::__cxx11 intentional?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #115: MINIFI-324 - More flexible uid generation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/115#discussion_r123745514
  
--- Diff: libminifi/src/utils/Id.cpp ---
@@ -0,0 +1,154 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "utils/Id.h"
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "core/logging/LoggerConfiguration.h"
+#include "utils/StringUtils.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+uint64_t timestamp = 
std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count();
+
+NonRepeatingStringGenerator::NonRepeatingStringGenerator() : 
prefix_((std::to_string(timestamp) + "-")) {}
+
+IdGenerator::IdGenerator() : implementation_(UUID_TIME_IMPL), 
logger_(logging::LoggerFactory::getLogger()), incrementor_(0) {
+}
+
+uint64_t IdGenerator::getDeviceSegmentFromString(const std::string& str, 
int numBits) {
+  uint64_t deviceSegment = 0;
+  for (int i = 0; i < str.length(); i++) {
+unsigned char c = toupper(str[i]);
+if (c >= '0' && c <= '9') {
+  deviceSegment = deviceSegment + (c - '0');
+} else if (c >= 'A' && c <= 'F') {
+  deviceSegment = deviceSegment + (c - 'A' + 10);
+} else {
+  logger_->log_error("Expected hex char (0-9, A-F).  Got %c.", c);
+}
+deviceSegment = deviceSegment << 4;
+  }
+  deviceSegment <<= 64 - (4 * (str.length() + 1));
+  deviceSegment >>= 64 - numBits;
+  logger_->log_debug("Using user defined device segment: %" PRIx64, 
deviceSegment);
+  deviceSegment <<= 64 - numBits;
+  return deviceSegment;
+}
+
+uint64_t IdGenerator::getRandomDeviceSegment(int numBits) {
+  uint64_t deviceSegment = 0;
+  uuid_t random_uuid;
+  for (int word = 0; word < 2; word++) {
+uuid_generate_random(random_uuid);
+for (int i = 0; i < 4; i++) {
+  deviceSegment += random_uuid[i];
+  deviceSegment <<= 8;
+}
+  }
+  deviceSegment >>= 64 - numBits;
+  logger_->log_debug("Using random device segment: %" PRIx64, 
deviceSegment);
+  deviceSegment <<= 64 - numBits;
+  return deviceSegment;
+}
+
+void IdGenerator::initialize(const std::shared_ptr & 
properties) {
+  std::string implementation_str;
+  implementation_ = UUID_TIME_IMPL;
+  if (properties->get("uid.implementation", implementation_str)) {
+std::transform(implementation_str.begin(), implementation_str.end(), 
implementation_str.begin(), ::tolower);
+if ("random" == implementation_str) {
+  logger_->log_debug("Using uuid_generate_random for uids.");
+  implementation_ = UUID_RANDOM_IMPL;
+} else if ("uuid_default" == implementation_str) {
+  logger_->log_debug("Using uuid_generate for uids.");
+  implementation_ = UUID_DEFAULT_IMPL;
+} else if ("minifi_uid" == implementation_str) {
+  logger_->log_debug("Using minifi uid implementation for uids");
+  implementation_ = MINIFI_UID_IMPL;
+
+  uint64_t timestamp = 
std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()).count();
+  int device_bits = 
properties->getInt("uid.minifi.device.segment.bits", 16);
+  std::string device_segment;
+  uint64_t prefix = timestamp;
+  if (device_bits > 0) {
+if (properties->get("uid.minifi.device.segment", device_segment)) {
+  prefix = getDeviceSegmentFromString(device_segment, device_bits);
+} else {
+  logger_->log_warn("uid.minifi.device.segment not sp

[GitHub] nifi-minifi-cpp pull request #115: MINIFI-324 - More flexible uid generation

2017-06-23 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/115#discussion_r123746263
  
--- Diff: README.md ---
@@ -326,26 +326,44 @@ Additionally, users can utilize the MiNiFi Toolkit 
Converter (version 0.0.1 - sc
 
 ### Http Configuration Listener
 
-Http Configuration Listener will pull flow file configuration from the 
remote command control server, 
+Http Configuration Listener will pull flow file configuration from the 
remote command control server,
 validate and apply the new flow configuration
 
-in minifi.properties 
+in minifi.properties
 
 nifi.configuration.listener.type=http
 nifi.configuration.listener.http.url=https://localhost:8080
-nifi.configuration.listener.pull.interval=1 sec 
+nifi.configuration.listener.pull.interval=1 sec
 nifi.configuration.listener.client.ca.certificate=./conf/nifi-cert.pem
 
 if you want to enable client certificate
 nifi.configuration.listener.need.ClientAuth=true
 nifi.configuration.listener.client.certificate=./conf/client.pem
 nifi.configuration.listener.client.private.key=./conf/client.key
 nifi.configuration.listener.client.pass.phrase=./conf/password
-  
+
+### UID generation
+
+MiNiFi need to generate many unique identifiers in the course of 
operations.  There are a few different uid implementations available that can 
be configured in minifi-uid.properties.
+
+Implementation for uid generation can be selected using the 
uid.implementation property values:
+1. time - use uuid_generate_time
+2. random - use uuid_generate_random
+3. uuid_default - use uuid_generate (will attempt to use 
uuid_generate_random and fall back to uuid_generate_time if no high quality 
randomness is available)
--- End diff --

I added a comment in Id.cpp to clarify the location in code I was referring 
to. Sorry for the confusion. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #98: MINIFI-37: Create a volatile repository and confi...

2017-05-22 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/98
  
@apiri This should be good once travis deems it so. I'm adding some 
information on the wiki on configuring this and other portions of MiNiFi cpp


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #102: MINIFI-207: Use recursive mutex that avoi...

2017-05-22 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/102

MINIFI-207: Use recursive mutex that avoid thread safety concerns

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-207

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/102.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #102






---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #101: MINIFI-325: Set stream_factory_ from FlowConfigu...

2017-05-20 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/101
  
Reviewers, I did not add a test, but will do so at the beginning of the 
week. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #101: MINIFI-325: Set stream_factory_ from Flow...

2017-05-20 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/101

MINIFI-325: Set stream_factory_ from FlowConfiguration constructor

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-325

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/101.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #101


commit 2a2d8eadf226df7dd967113663596d498c18d100
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-05-20T20:41:25Z

MINIFI-325: Set stream_factory_ from FlowConfiguration constructor




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118830045
  
--- Diff: libminifi/src/Configure.cpp ---
@@ -71,12 +71,31 @@ const char *Configure::nifi_security_client_pass_phrase 
=
 "nifi.security.client.pass.phrase";
 const char *Configure::nifi_security_client_ca_certificate =
 "nifi.security.client.ca.certificate";
+const char *Configure::nifi_configuration_listener_pull_interval =
+"nifi.configuration.listener.pull.interval";
+const char *Configure::nifi_configuration_listener_http_url =
+"nifi.configuration.listener.http.url";
+const char *Configure::nifi_configuration_listener_rest_url =
+"nifi.configuration.listener.rest.url";
+const char *Configure::nifi_configuration_listener_type =
+"nifi.configuration.listener.type";
+const char *Configure::nifi_configuration_listener_need_ClientAuth =
+"nifi.configuration.listener.need.ClientAuth";
+const char *Configure::nifi_configuration_listener_client_certificate =
--- End diff --

there are a lot of duplicate options here. it's possible that users will 
want to use different certs for different applications, which we should 
support; however, if consumers are using the same certificate perhaps we could 
make the configuration less onerous on them by tying configs together. this may 
make the eventual C2 impl easier to configure as well. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118830430
  
--- Diff: libminifi/src/ConfigurationListener.cpp ---
@@ -0,0 +1,130 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "ConfigurationListener.h"
+#include "FlowController.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+
+void ConfigurationListener::start() {
+  if (running_)
+return;
+
+  pull_interval_ = 60 * 1000;
+  std::string value;
+  // grab the value for configuration
+  if (configure_->get(Configure::nifi_configuration_listener_pull_interval,
+  value)) {
+core::TimeUnit unit;
+if (core::Property::StringToTime(value, pull_interval_, unit)
+&& core::Property::ConvertTimeUnitToMS(pull_interval_, unit,
+pull_interval_)) {
+  logger_->log_info("Configuration Listener pull interval: [%d] ms",
+   pull_interval_);
+}
+  }
+
+  std::string clientAuthStr;
+  if 
(configure_->get(Configure::nifi_configuration_listener_need_ClientAuth, 
clientAuthStr)) {
+
org::apache::nifi::minifi::utils::StringUtils::StringToBool(clientAuthStr, 
this->need_client_certificate_);
+  }
+
+  if (configure_->get(
+  Configure::nifi_configuration_listener_client_ca_certificate,
+  this->ca_certificate_)) {
+logger_->log_info("Configuration Listener CA certificates: [%s]",
+this->ca_certificate_.c_str());
+  }
+
+  if (this->need_client_certificate_) {
+std::string passphrase_file;
+
+if (!(configure_->get(
+Configure::nifi_configuration_listener_client_certificate, 
this->certificate_)
+&& 
configure_->get(Configure::nifi_configuration_listener_private_key,
+this->private_key_))) {
+  logger_->log_error(
+  "Certificate and Private Key PEM file not configured for 
configuration listener, error: %s.",
+  std::strerror(errno));
+}
+
+if (configure_->get(
+Configure::nifi_configuration_listener_client_pass_phrase,
+passphrase_file)) {
+  // load the passphase from file
+  std::ifstream file(passphrase_file.c_str(), std::ifstream::in);
+  if (file.good()) {
+this->passphrase_.assign((std::istreambuf_iterator(file)),
+std::istreambuf_iterator());
+file.close();
+  }
+}
+
+logger_->log_info("Configuration Listener certificate: [%s], private 
key: [%s], passphrase file: [%s]",
+this->certificate_.c_str(), this->private_key_.c_str(), 
passphrase_file.c_str());
+  }
+
+  thread_ = std::thread(::threadExecutor, this);
+  thread_.detach();
+  running_ = true;
+  logger_->log_info("%s ConfigurationListener Thread Start", 
type_.c_str());
+}
+
+void ConfigurationListener::stop() {
+  if (!running_)
+return;
+  running_ = false;
+  if (thread_.joinable())
+thread_.join();
+  logger_->log_info("%s ConfigurationListener Thread Stop", type_.c_str());
+}
+
+void ConfigurationListener::run() {
+  int64_t interval = 0;
+  while (running_) {
+std::this_thread::sleep_for(std::chrono::milliseconds(100));
--- End diff --

is the 100ms sleep to allow this object to check for interruptibility? If 
that is the case could you use a condition variable with a wait_for and avoid 
the checks, sleep, and regular wakeup? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118822078
  
--- Diff: libminifi/src/FlowController.cpp ---
@@ -90,8 +91,18 @@ FlowController::FlowController(
   initialized_ = false;
   root_ = nullptr;
 
+  memset(serial_number_, 0, 6);
+
   protocol_ = new FlowControlProtocol(this, configure);
 
+  std::string listenerType;
+  // grab the value for configuration
+  if (configure->get(Configure::nifi_configuration_listener_type, 
listenerType)) {
+if (listenerType == "http") {
+  this->http_configuration_listener_ = 
std::unique_ptr(new 
minifi::HttpConfigurationListener(this, configure));
--- End diff --

You should be passing shared_from_this() to the instantiated object to 
avoid issues with accessing the shared pointer as a raw pointer. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118829992
  
--- Diff: libminifi/src/FlowController.cpp ---
@@ -163,6 +174,31 @@ FlowController::~FlowController() {
   provenance_repo_ = nullptr;
 }
 
+bool FlowController::applyConfiguration(std::string ) {
--- End diff --

I have a general comment. I'm putting it here because as an alternative 
approach could we put the listener changes into the FlowConfiguration and 
potentially have a callback. this might get rid of the need for the forward 
declaration and for the listener to know anything about the flow controller. it 
would be nice to abstract this into some kind of c2 suite so that we an 
encapsulate this and everything else into a package that's homogenous. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118822142
  
--- Diff: libminifi/include/FlowController.h ---
@@ -131,9 +134,50 @@ class FlowController : public 
core::controller::ControllerServiceProvider,
   root_->updatePropertyValue(processorName, propertyName, 
propertyValue);
   }
 
-  // set 8 bytes SerialNumber
-  virtual void setSerialNumber(uint8_t *number) {
-protocol_->setSerialNumber(number);
+  // set 6 bytes SerialNumber
+  void setSerialNumber(uint8_t *number) {
+memcpy(serial_number_, number, 6);
+  }
+
+  //get serial number
+  uint8_t *getSerialNumber() {
+return serial_number_;
+  }
+
+  // get serial number as string
+  std::string getSerialNumberStr() {
+char str[18];
+char *strPtr = [0];
+
+for (int i = 0; i < 6; i++) {
--- End diff --

if you use a string instead of uint8_t this can become
```
std::stringstream hex;
hex >> std::hex >> serial_number_;
return hex.str();
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118822157
  
--- Diff: libminifi/include/HttpConfigurationListener.h ---
@@ -0,0 +1,102 @@
+/**
+ * HttpConfigurationListener class declaration
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __HTTP_CONFIGURATION_LISTENER__
+#define __HTTP_CONFIGURATION_LISTENER__
+
+#include 
+#include "core/Core.h"
+#include "core/Property.h"
+#include "ConfigurationListener.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+/**
+ * HTTP Response object
+ */
+struct HTTPRequestResponse {
--- End diff --

Does it make sense to make some of the InvokeHTTP code rely on this and 
break it into a centralized place for them to use?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118822209
  
--- Diff: libminifi/src/FlowController.cpp ---
@@ -175,7 +211,7 @@ void FlowController::stop(bool force) {
 this->flow_file_repo_->stop();
 this->provenance_repo_->stop();
 // Wait for sometime for thread stop
-std::this_thread::sleep_for(std::chrono::milliseconds(1000));
+std::this_thread::sleep_for(std::chrono::milliseconds(3000));
--- End diff --

the sleep here seems arbitrary. why was 3 seconds chosen?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118822113
  
--- Diff: libminifi/include/ConfigurationListener.h ---
@@ -0,0 +1,121 @@
+/**
+ * ConfigurationListener class declaration
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __CONFIGURATION_LISTENER__
+#define __CONFIGURATION_LISTENER__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "yaml-cpp/yaml.h"
+#include "core/Core.h"
+#include "core/Property.h"
+#include "properties/Configure.h"
+#include "core/logging/Logger.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+// Forwarder declaration
+class FlowController;
+// ConfigurationListener Class
+class ConfigurationListener {
+public:
+
+  // Constructor
+  /*!
+   * Create a new processor
+   */
+  ConfigurationListener(FlowController *controller,
+  std::shared_ptr configure, std::string type) :
+  connect_timeout_(2), read_timeout_(2), type_(type), 
configure_(
+  configure), controller_(controller), 
need_client_certificate_(false) {
+logger_ = logging::Logger::getLogger();
+running_ = false;
+  }
+  // Destructor
+  virtual ~ConfigurationListener() {
+stop();
+  }
+
+  // Start the thread
+  void start();
+  // Stop the thread
+  void stop();
+  // whether the thread is enable
+  bool isRunning() {
+return running_;
+  }
+  // pull the new configuration from the remote host
+  virtual bool pullConfiguration(std::string ) {
+return false;
+  }
+
+protected:
+
+  // Run function for the thread
+  void run();
+
+  // Run function for the thread
+  void threadExecutor() {
+run();
+  }
+
+  // Mutex for protection
+  std::mutex mutex_;
+  // thread
+  std::thread thread_;
+  // whether the thread is running
+  bool running_;
+
+  // url
+  std::string url_;
+  // connection timeout
+  int64_t connect_timeout_;
+  // read timeout.
+  int64_t read_timeout_;
+  // pull interval
+  int64_t pull_interval_;
+  // type (http/rest)
+  std::string type_;
+  // last applied configuration
+  std::string lastAppliedConfiguration;
+
+  std::shared_ptr configure_;
+  std::shared_ptr logger_;
+  FlowController *controller_;
+
+  bool need_client_certificate_;
--- End diff --

this is some of the same code used by other classes. Does it make sense to 
encapsulate this elsewhere to minimize duplicate code? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118822084
  
--- Diff: libminifi/src/FlowController.cpp ---
@@ -90,8 +91,18 @@ FlowController::FlowController(
   initialized_ = false;
   root_ = nullptr;
 
+  memset(serial_number_, 0, 6);
--- End diff --

if you use a string this op is unnecessary. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118822171
  
--- Diff: libminifi/src/ConfigurationListener.cpp ---
@@ -0,0 +1,130 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "ConfigurationListener.h"
+#include "FlowController.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+
+void ConfigurationListener::start() {
+  if (running_)
+return;
+
+  pull_interval_ = 60 * 1000;
+  std::string value;
+  // grab the value for configuration
+  if (configure_->get(Configure::nifi_configuration_listener_pull_interval,
+  value)) {
+core::TimeUnit unit;
+if (core::Property::StringToTime(value, pull_interval_, unit)
+&& core::Property::ConvertTimeUnitToMS(pull_interval_, unit,
+pull_interval_)) {
+  logger_->log_info("Configuration Listener pull interval: [%d] ms",
+   pull_interval_);
+}
+  }
+
+  std::string clientAuthStr;
+  if 
(configure_->get(Configure::nifi_configuration_listener_need_ClientAuth, 
clientAuthStr)) {
+
org::apache::nifi::minifi::utils::StringUtils::StringToBool(clientAuthStr, 
this->need_client_certificate_);
+  }
+
+  if (configure_->get(
+  Configure::nifi_configuration_listener_client_ca_certificate,
+  this->ca_certificate_)) {
+logger_->log_info("Configuration Listener CA certificates: [%s]",
+this->ca_certificate_.c_str());
+  }
+
+  if (this->need_client_certificate_) {
+std::string passphrase_file;
+
+if (!(configure_->get(
+Configure::nifi_configuration_listener_client_certificate, 
this->certificate_)
+&& 
configure_->get(Configure::nifi_configuration_listener_private_key,
+this->private_key_))) {
+  logger_->log_error(
+  "Certificate and Private Key PEM file not configured for 
configuration listener, error: %s.",
+  std::strerror(errno));
+}
+
+if (configure_->get(
+Configure::nifi_configuration_listener_client_pass_phrase,
+passphrase_file)) {
+  // load the passphase from file
+  std::ifstream file(passphrase_file.c_str(), std::ifstream::in);
+  if (file.good()) {
+this->passphrase_.assign((std::istreambuf_iterator(file)),
+std::istreambuf_iterator());
+file.close();
+  }
+}
+
+logger_->log_info("Configuration Listener certificate: [%s], private 
key: [%s], passphrase file: [%s]",
+this->certificate_.c_str(), this->private_key_.c_str(), 
passphrase_file.c_str());
+  }
+
+  thread_ = std::thread(::threadExecutor, this);
--- End diff --

Is there a way we can tie this into the scheduling agents?  They have a 
threadpool in there 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118830027
  
--- Diff: libminifi/src/core/yaml/YamlConfiguration.cpp ---
@@ -32,16 +32,25 @@ namespace core {
 core::ProcessGroup *YamlConfiguration::parseRootProcessGroupYaml(
 YAML::Node rootFlowNode) {
   uuid_t uuid;
+  int64_t version = 0;
 
   checkRequiredField(, "name", 
CONFIG_YAML_REMOTE_PROCESS_GROUP_KEY);
   std::string flowName = rootFlowNode["name"].as();
   std::string id = getOrGenerateId();
   uuid_parse(id.c_str(), uuid);
 
-  logger_->log_debug("parseRootProcessGroup: id => [%s], name => [%s]", id,
- flowName);
+  if (rootFlowNode["version"]) {
+std::string value = rootFlowNode["version"].as();
+if (core::Property::StringToInt(value, version)) {
+  logger_->log_debug("parseRootProcessorGroup: version => [%d]", 
version);
+}
+  }
+
+  logger_->log_debug(
--- End diff --

thanks for using the string. i saw a few places where it was id.c_str() if 
you could make them consistent that would be awesome. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118822167
  
--- Diff: libminifi/src/ConfigurationListener.cpp ---
@@ -0,0 +1,130 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "ConfigurationListener.h"
+#include "FlowController.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+
+void ConfigurationListener::start() {
+  if (running_)
+return;
+
+  pull_interval_ = 60 * 1000;
+  std::string value;
+  // grab the value for configuration
+  if (configure_->get(Configure::nifi_configuration_listener_pull_interval,
+  value)) {
+core::TimeUnit unit;
+if (core::Property::StringToTime(value, pull_interval_, unit)
+&& core::Property::ConvertTimeUnitToMS(pull_interval_, unit,
+pull_interval_)) {
+  logger_->log_info("Configuration Listener pull interval: [%d] ms",
+   pull_interval_);
+}
+  }
+
+  std::string clientAuthStr;
+  if 
(configure_->get(Configure::nifi_configuration_listener_need_ClientAuth, 
clientAuthStr)) {
+
org::apache::nifi::minifi::utils::StringUtils::StringToBool(clientAuthStr, 
this->need_client_certificate_);
+  }
+
+  if (configure_->get(
+  Configure::nifi_configuration_listener_client_ca_certificate,
+  this->ca_certificate_)) {
+logger_->log_info("Configuration Listener CA certificates: [%s]",
--- End diff --

you only need to do this->ca_certificate. we've made convenience functions 
in the logger to allow for this. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118821534
  
--- Diff: libminifi/include/FlowController.h ---
@@ -131,9 +134,50 @@ class FlowController : public 
core::controller::ControllerServiceProvider,
   root_->updatePropertyValue(processorName, propertyName, 
propertyValue);
   }
 
-  // set 8 bytes SerialNumber
-  virtual void setSerialNumber(uint8_t *number) {
-protocol_->setSerialNumber(number);
+  // set 6 bytes SerialNumber
+  void setSerialNumber(uint8_t *number) {
--- End diff --

why not deal with the number as a string since you are returning it as a 
string, below?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118822193
  
--- Diff: libminifi/include/core/repository/FlowFileRepository.h ---
@@ -36,7 +36,7 @@ namespace repository {
 #define FLOWFILE_REPOSITORY_DIRECTORY "./flowfile_repository"
 #define MAX_FLOWFILE_REPOSITORY_STORAGE_SIZE (10*1024*1024) // 10M
 #define MAX_FLOWFILE_REPOSITORY_ENTRY_LIFE_TIME (60) // 10 minute
-#define FLOWFILE_REPOSITORY_PURGE_PERIOD (2500) // 2500 msec
+#define FLOWFILE_REPOSITORY_PURGE_PERIOD (2000) // 2000 msec
--- End diff --

why was this reduced?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118830048
  
--- Diff: libminifi/include/ConfigurationListener.h ---
@@ -0,0 +1,121 @@
+/**
+ * ConfigurationListener class declaration
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __CONFIGURATION_LISTENER__
+#define __CONFIGURATION_LISTENER__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "yaml-cpp/yaml.h"
+#include "core/Core.h"
+#include "core/Property.h"
+#include "properties/Configure.h"
+#include "core/logging/Logger.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+// Forwarder declaration
+class FlowController;
+// ConfigurationListener Class
+class ConfigurationListener {
+public:
+
+  // Constructor
+  /*!
+   * Create a new processor
+   */
+  ConfigurationListener(FlowController *controller,
+  std::shared_ptr configure, std::string type) :
+  connect_timeout_(2), read_timeout_(2), type_(type), 
configure_(
+  configure), controller_(controller), 
need_client_certificate_(false) {
+logger_ = logging::Logger::getLogger();
+running_ = false;
+  }
+  // Destructor
+  virtual ~ConfigurationListener() {
+stop();
+  }
+
+  // Start the thread
+  void start();
+  // Stop the thread
+  void stop();
+  // whether the thread is enable
+  bool isRunning() {
+return running_;
+  }
+  // pull the new configuration from the remote host
+  virtual bool pullConfiguration(std::string ) {
+return false;
+  }
+
+protected:
+
+  // Run function for the thread
+  void run();
+
+  // Run function for the thread
+  void threadExecutor() {
+run();
+  }
+
+  // Mutex for protection
+  std::mutex mutex_;
+  // thread
+  std::thread thread_;
+  // whether the thread is running
+  bool running_;
--- End diff --

^ i'm worried about cache coherency here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #107: Configuration listener

2017-05-27 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/107#discussion_r118821541
  
--- Diff: libminifi/include/FlowController.h ---
@@ -131,9 +134,50 @@ class FlowController : public 
core::controller::ControllerServiceProvider,
   root_->updatePropertyValue(processorName, propertyName, 
propertyValue);
   }
 
-  // set 8 bytes SerialNumber
-  virtual void setSerialNumber(uint8_t *number) {
-protocol_->setSerialNumber(number);
+  // set 6 bytes SerialNumber
+  void setSerialNumber(uint8_t *number) {
+memcpy(serial_number_, number, 6);
+  }
+
+  //get serial number
+  uint8_t *getSerialNumber() {
--- End diff --

why is this necessary?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #106: MINIFI-328: Change source of the minifi I...

2017-05-25 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/106

MINIFI-328: Change source of the minifi InvokeHTTP Get scripts

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-328

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/106.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #106


commit 7f9b693ffdaee50ef01e1b04f8018b7575bfaf44
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-05-25T18:01:09Z

MINIFI-328: Change source of the minifi InvokeHTTP Get scripts




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #97: MINIFI-289: Update test folder to apply linter an...

2017-05-19 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/97
  
@apiri rebase complete. Waiting on travis to prove me right/wrong...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #98: MINIFI-37: Create a volatile repository and confi...

2017-05-18 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/98
  
@apiri after addressing your comments, above, I ran tests which made me 
rethink part of how this was tested. To cover my bases I'm going to test a 
little more and comment when it's ready again. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #100: MINIFI-320: Move instantion of OpenSSL to...

2017-05-18 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/100

MINIFI-320: Move instantion of OpenSSL to a singleton that's

lazily created. Added test. In order to make the test work as
expected I had to update thread pool to use a non blocking queue.

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-320

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/100.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #100


commit 68d7d6f14c96a73f89dead04e37895af6e1957fc
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-05-18T18:21:16Z

MINIFI-320: Move instantion of OpenSSL to a singleton that's
lazily created. Added test. In order to make the test work as
expected I had to update thread pool to use a non blocking queue.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #100: MINIFI-320: Move instantiation of OpenSSL...

2017-05-18 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/100#discussion_r117346816
  
--- Diff: libminifi/include/utils/ThreadPool.h ---
@@ -189,7 +195,8 @@ class ThreadPool {
 // atomic running boolean
   std::atomic running_;
 // worker queue of worker objects
-  std::queue<Worker> worker_queue_;
+  //std::queue<Worker> worker_queue_;
--- End diff --

remove comment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #100: MINIFI-320: Move instantiation of OpenSSL...

2017-05-18 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/100#discussion_r117346914
  
--- Diff: libminifi/test/unit/SocketTests.cpp ---
@@ -183,3 +196,42 @@ TEST_CASE("TestSocketWriteTestAfterClose", 
"[TestSocket6]") {
   server.closeStream();
 
 }
+
+std::atomic counter;
+std::mt19937_64 seed { std::random_device { }() };
+bool createSocket() {
+  int mine = counter++;
+  std::shared_ptr configuration = std::make_shared<
+  minifi::Configure>();
+
+  std::uniform_int_distribution<> distribution { 10, 100 };
+  std::this_thread::sleep_for(std::chrono::milliseconds { 
distribution(seed) });
+
+  for (int i = 0; i < 50; i++) {
+std::shared_ptr socketA = std::make_shared(
+configuration);
+socketA->initialize();
+  }
+
+  return true;
+}
--- End diff --

add some comments to why this exists. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #100: MINIFI-320: Move instantiation of OpenSSL to a s...

2017-05-19 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/100
  
@apiri this PR is ready for review. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #109: MINIFI-330: convert const char* to std::s...

2017-06-01 Thread phrocker
Github user phrocker closed the pull request at:

https://github.com/apache/nifi-minifi-cpp/pull/109


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #109: MINIFI-330: convert const char* to std::string

2017-06-01 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/109
  
Rolling into MINIFI-249


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #109: MINIFI-330: convert const char* to std::s...

2017-05-31 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/109

MINIFI-330: convert const char* to std::string

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-330

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/109.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #109


commit 07e1d7ce207706d5c53822f4b1a9674f4e28decc
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-05-31T23:49:14Z

MINIFI-330: convert const char* to std::string




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #112: MINIFI-262: Configuration Listener

2017-06-07 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/112
  
+1 approved per evaluation in #107 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #107: MINIFI-262: Configuration listener

2017-06-07 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/107
  
+1 

@benqiu2016 Thanks for the update. Would you mind squashing your commits 
with a message you would like to include before we merge it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #110: MINIFI-249: Update prov repo to better ab...

2017-06-01 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/110

MINIFI-249: Update prov repo to better abstract deser.

Deserialization and serialization are better abstracted
into SerializableComponent allowing us to use all repos with
the same [de]serialization interfaces.

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-249

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/110.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #110


commit a07533927baa7a156c2b8a375702ff01992596a3
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-05-24T18:39:08Z

MINIFI-249: Update prov repo to better abstract deser.

Deserialization and serialization are better abstracted
into SerializableComponent allowing us to use all repos with
the same [de]serialization interfaces.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #110: MINIFI-249: Update prov repo to better abstract ...

2017-06-01 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/110
  
I will be self reviewing this in a bit. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #110: MINIFI-249: Update prov repo to better abstract ...

2017-06-01 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/110
  
There are travis failures. Will address. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #110: MINIFI-249: Update prov repo to better abstract ...

2017-06-06 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/110
  
Adding one more commit to close this out. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #110: MINIFI-249: Update prov repo to better abstract ...

2017-06-14 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/110
  
As a general comment, let's have the ability to minimize locking and not 
minimize locking in volatile content repository.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #86: MINIFI-282: Move Socket tests and remove includes

2017-04-29 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/86
  
@apiri yeah sorry it was not intended to take the place of #84 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #90: MINIFI-294 Required vs. optional fields in...

2017-05-04 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/90#discussion_r114745591
  
--- Diff: libminifi/src/core/yaml/YamlConfiguration.cpp ---
@@ -417,29 +405,27 @@ void YamlConfiguration::parseConnectionYaml(
 
 
 // Configure connection source
-auto rawRelationship = connectionNode["source relationship name"]
-.as();
+checkRequiredField(, "source relationship name", 
CONFIG_YAML_CONNECTIONS_KEY);
+auto rawRelationship = connectionNode["source relationship 
name"].as();
 core::Relationship relationship(rawRelationship, "");
-logger_->log_debug(
-"parseConnection: relationship => [%s]", rawRelationship);
+logger_->log_debug("parseConnection: relationship => [%s]", 
rawRelationship);
 if (connection) {
   connection->setRelationship(relationship);
 }
 
 uuid_t srcUUID;
-std::string connectionSrcProcName = connectionNode["source name"]
-.as();
+
 if (connectionNode["source id"]) {
-  std::string connectionSrcProcId = connectionNode["source id"]
-  .as();
+  std::string connectionSrcProcId = connectionNode["source 
id"].as();
 
   uuid_parse(connectionSrcProcId.c_str(), srcUUID);
 } else {
   // if we don't have a source id, try harder to resolve the 
source processor.
   // config schema v2 will make this unnecessary
+  checkRequiredField(, "source name", 
CONFIG_YAML_CONNECTIONS_KEY);
--- End diff --

@kevdoran I like that you documented your assumption. Could you augment 
that comment with a log message affirming the decision of the code so that we 
could better debug "how and why?"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp issue #92: MINIFI-299: Update Readme.

2017-05-05 Thread phrocker
Github user phrocker commented on the issue:

https://github.com/apache/nifi-minifi-cpp/pull/92
  
thanks @kevdoran 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #92: MINIFI-299: Update Readme.

2017-05-05 Thread phrocker
GitHub user phrocker opened a pull request:

https://github.com/apache/nifi-minifi-cpp/pull/92

MINIFI-299: Update Readme.

1) Update caveat section
2) Update list of processors
3) Correct some inconsistencies.

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 MINIFI- 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 master)?

- [ ] 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFI-299

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi-minifi-cpp/pull/92.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #92


commit 96e4fdfc8fcd81a1aad93d4bb4af1db1c87decbf
Author: Marc Parisi <phroc...@apache.org>
Date:   2017-05-05T17:03:46Z

MINIFI-299: Update Readme.

1) Update caveat section
2) Update list of processors
3) Correct some inconsistencies.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #91: MINIFI-258 - Removing Configure, StreamFac...

2017-05-04 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/91#discussion_r114861465
  
--- Diff: libminifi/include/core/FlowConfiguration.h ---
@@ -55,8 +56,9 @@ class FlowConfiguration : public CoreComponent {
* Constructor that will be used for configuring
* the flow controller.
*/
-  FlowConfiguration(std::shared_ptr repo,
-std::shared_ptr flow_file_repo,
+  FlowConfiguration(const std::shared_ptr ,
--- End diff --

Same as flow controller


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #91: MINIFI-258 - Removing Configure, StreamFac...

2017-05-04 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/91#discussion_r114861428
  
--- Diff: libminifi/include/core/ConfigurationFactory.h ---
@@ -30,25 +30,29 @@ namespace core {
 
 template
 typename std::enable_if::value, T*>::type instantiate(
-std::shared_ptr repo,
-std::shared_ptr flow_file_repo, const std::string 
path) {
+const std::shared_ptr ,
+const std::shared_ptr _file_repo, const 
std::string path) {
   throw std::runtime_error("Cannot instantiate class");
 }
 
 template
 typename std::enable_if<class_operations::value, T*>::type instantiate(
-std::shared_ptr repo,
-std::shared_ptr flow_file_repo, const std::string 
path) {
-  return new T(repo, flow_file_repo, path);
+const std::shared_ptr ,
+const std::shared_ptr _file_repo,
+const std::shared_ptr _factory,
+const std::string path) {
+  return new T(repo, flow_file_repo, stream_factory, path);
 }
 
 /**
  * Configuration factory is used to create a new FlowConfiguration
  * object.
  */
 std::unique_ptr createFlowConfiguration(
--- End diff --

Same as flow controller. The ones in instantiate, above, are fine. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #91: MINIFI-258 - Removing Configure, StreamFac...

2017-05-04 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/91#discussion_r114861939
  
--- Diff: libminifi/include/core/yaml/YamlConfiguration.h ---
@@ -41,10 +42,12 @@ namespace core {
 class YamlConfiguration : public FlowConfiguration {
 
  public:
-  YamlConfiguration(std::shared_ptr repo,
-std::shared_ptr flow_file_repo,
+  YamlConfiguration(const std::shared_ptr ,
--- End diff --

might have to do the same here to avoid removing const ( which you can do 
obviously ) 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #91: MINIFI-258 - Removing Configure, StreamFac...

2017-05-04 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/91#discussion_r114860818
  
--- Diff: libminifi/include/FlowController.h ---
@@ -64,8 +64,9 @@ class FlowController : public core::CoreComponent {
   /**
* Flow controller constructor
*/
-  FlowController(std::shared_ptr provenance_repo,
- std::shared_ptr flow_file_repo,
+  FlowController(const std::shared_ptr _repo,
--- End diff --

The reason these were pass by value was that we wanted to ensure that we 
increment the ref counter for flow controller. Thus we won't deallocate memory 
during its scope. It's a safety mechanism and more importantly indicates that 
these need to survive FlowController


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #91: MINIFI-258 - Removing Configure, StreamFac...

2017-05-04 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/91#discussion_r114864639
  
--- Diff: libminifi/include/EventDrivenSchedulingAgent.h ---
@@ -38,8 +38,8 @@ class EventDrivenSchedulingAgent : public 
ThreadedSchedulingAgent {
   /*!
* Create a new processor
*/
-  EventDrivenSchedulingAgent(std::shared_ptr repo)
-  : ThreadedSchedulingAgent(repo) {
+  EventDrivenSchedulingAgent(const std::shared_ptr 
, const std::shared_ptr )
--- End diff --

These schedulers stop in a way in which we don't guarantee threads to stop. 
We wait in flow controller 1s to stop threads, thus we can't be 100% certain 
that the scheduled threads have stopped. They "probably" did, but we likely 
want to keep this pass by value to ensure that we are guaranteed shared 
ownership of the shared repo. Since the threads are daemon threads, they will 
stop eventually, but we probably want to avoid the error of a dereferencing 
deleted memory if we can to aid in eventual recoverability ( if that's a thing 
).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #91: MINIFI-258 - Removing Configure, StreamFac...

2017-05-04 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/91#discussion_r114863373
  
--- Diff: libminifi/include/FlowController.h ---
@@ -64,8 +64,9 @@ class FlowController : public core::CoreComponent {
   /**
* Flow controller constructor
*/
-  FlowController(std::shared_ptr provenance_repo,
- std::shared_ptr flow_file_repo,
+  FlowController(const std::shared_ptr _repo,
--- End diff --

we could move the original std shared ptr to subsume ownership, but I'm not 
sure we want that. We want to say we're sharing ownership with this object and 
thus this object ( FlowController ) helps in determining the lifetime of 
shared_ptr. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #85: MINIFI-275 Bugfix for YAML Configs without...

2017-05-02 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/85#discussion_r114137733
  
--- Diff: libminifi/test/unit/YamlCongifurationTests.cpp ---
@@ -0,0 +1,182 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include 
+#include "core/yaml/YamlConfiguration.h"
+#include "../TestBase.h"
+
+static const std::shared_ptr TEST_PROV_REPO = 
core::createRepository("provenancerepository", true);
+static const std::shared_ptr TEST_FF_REPO = 
core::createRepository("flowfilerepository", true);
+
+TEST_CASE("Test YAML Config 1", "[testyamlconfig1]") {
+
+  static const std::string TEST_YAML_WITHOUT_IDS = "MiNiFi Config Version: 
1\n"
--- End diff --

You can add this into the resource directory -- I'm renaming this to 
resources in a subsequent PR, but there's no need to maintain this text here. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #85: MINIFI-275 Bugfix for YAML Configs without...

2017-05-02 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/85#discussion_r114138536
  
--- Diff: libminifi/src/core/yaml/YamlConfiguration.cpp ---
@@ -491,14 +510,18 @@ void YamlConfiguration::parsePortYaml(YAML::Node 
*portNode,
 
   YAML::Node inputPortsObj = portNode->as();
 
-  // generate the random UIID
-  uuid_generate(uuid);
-
-  auto portId = inputPortsObj["id"].as();
+  // Check for required fields
+  checkRequiredField(, "name");
   auto nameStr = inputPortsObj["name"].as();
+  checkRequiredField(, "id", "The field 'id' is required for 
"
+  "the port named '" + nameStr + "' in the YAML Config. If this port "
+  "is specificy an input port for a NiFi Remote Process Group, the 
port "
+  "id should match the id of of the input port in the NiFi 
configuration. "
+  "This is a UUID of the format 
----.");
+  auto portId = inputPortsObj["id"].as();
   uuid_parse(portId.c_str(), uuid);
 
-  port = new minifi::RemoteProcessorGroupPort(nameStr.c_str(), uuid);
+  port = new minifi::RemoteProcessorGroupPort(nameStr, uuid);
 
   processor = (std::shared_ptr) port;
--- End diff --

we should fix this while we're here. we shouldn't cast a raw pointer to a 
shared pointer. That can have unintended consequences. I didn't catch this when 
making changes, apparently. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #85: MINIFI-275 Bugfix for YAML Configs without...

2017-05-02 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/85#discussion_r114137974
  
--- Diff: libminifi/src/core/yaml/YamlConfiguration.cpp ---
@@ -538,14 +561,62 @@ void YamlConfiguration::parsePropertiesNodeYaml(
   std::string rawValueString = propertyValueNode.as();
   if (!processor->setProperty(propertyName, rawValueString)) {
 logger_->log_warn(
-"Received property %s with value %s but is not one of the 
properties for %s",
-propertyName.c_str(), rawValueString.c_str(),
-processor->getName().c_str());
+"Received property %s with value %s but it is not one of the 
properties for %s",
+propertyName,
+rawValueString,
+processor->getName());
   }
 }
   }
 }
 
+std::string YamlConfiguration::getOrGenerateId(
+YAML::Node *yamlNode,
+std::string idField) {
+  std::string id;
+  YAML::Node node = yamlNode->as();
+
+  if (node[idField]) {
+if (YAML::NodeType::Scalar == node[idField].Type()) {
+  id = node[idField].as();
+} else {
+  throw std::invalid_argument(
+  "getOrGenerateId: idField is expected to reference YAML::Node "
+  "of YAML::NodeType::Scalar.");
+}
+  } else {
+uuid_t uuid;
+uuid_generate(uuid);
+char uuid_str[37];
+uuid_unparse(uuid, uuid_str);
+id = uuid_str;
+logger_->log_debug("Generating random ID: id => [%s]", id);
+  }
+  return id;
+}
+
+void YamlConfiguration::checkRequiredField(
--- End diff --

Perhaps I should rephrase this to say that we can also get an InvalidNode 
exception. I think this occurs when the node exists check occurs, and when the 
current node isn't valid. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #85: MINIFI-275 Bugfix for YAML Configs without...

2017-05-02 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/85#discussion_r114138774
  
--- Diff: libminifi/include/core/yaml/YamlConfiguration.h ---
@@ -88,10 +102,17 @@ class YamlConfiguration : public FlowConfiguration {
   void parseRemoteProcessGroupYaml(YAML::Node *node,
core::ProcessGroup * parent);
   // Process Provenance Report YAML
-  void parseProvenanceReportingYaml(YAML::Node *reportNode, 
core::ProcessGroup * parentGroup);
+  void parseProvenanceReportingYaml(YAML::Node *reportNode,
+core::ProcessGroup * parentGroup);
   // Parse Properties Node YAML for a processor
   void parsePropertiesNodeYaml(YAML::Node *propertiesNode,
std::shared_ptr processor);
+  // Helper function to get or generate a component's id field
+  std::string getOrGenerateId(YAML::Node *yamlNode, std::string idField = 
"id");
+  // Get or generate a component's id field
--- End diff --

I'm not sure this comment matches. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #85: MINIFI-275 Bugfix for YAML Configs without...

2017-05-02 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/85#discussion_r114137339
  
--- Diff: libminifi/src/core/yaml/YamlConfiguration.cpp ---
@@ -491,14 +510,18 @@ void YamlConfiguration::parsePortYaml(YAML::Node 
*portNode,
 
   YAML::Node inputPortsObj = portNode->as();
 
-  // generate the random UIID
-  uuid_generate(uuid);
-
-  auto portId = inputPortsObj["id"].as();
+  // Check for required fields
+  checkRequiredField(, "name");
   auto nameStr = inputPortsObj["name"].as();
+  checkRequiredField(, "id", "The field 'id' is required for 
"
+  "the port named '" + nameStr + "' in the YAML Config. If this port "
+  "is specificy an input port for a NiFi Remote Process Group, the 
port "
--- End diff --

typo


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #85: MINIFI-275 Bugfix for YAML Configs without...

2017-05-02 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/85#discussion_r114138828
  
--- Diff: libminifi/include/core/yaml/YamlConfiguration.h ---
@@ -88,10 +102,17 @@ class YamlConfiguration : public FlowConfiguration {
   void parseRemoteProcessGroupYaml(YAML::Node *node,
core::ProcessGroup * parent);
   // Process Provenance Report YAML
-  void parseProvenanceReportingYaml(YAML::Node *reportNode, 
core::ProcessGroup * parentGroup);
+  void parseProvenanceReportingYaml(YAML::Node *reportNode,
+core::ProcessGroup * parentGroup);
   // Parse Properties Node YAML for a processor
   void parsePropertiesNodeYaml(YAML::Node *propertiesNode,
std::shared_ptr processor);
+  // Helper function to get or generate a component's id field
+  std::string getOrGenerateId(YAML::Node *yamlNode, std::string idField = 
"id");
+  // Get or generate a component's id field
--- End diff --

Let's make a habit of using const std::string 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #85: MINIFI-275 Bugfix for YAML Configs without...

2017-05-02 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/85#discussion_r114137644
  
--- Diff: libminifi/src/core/yaml/YamlConfiguration.cpp ---
@@ -538,14 +561,62 @@ void YamlConfiguration::parsePropertiesNodeYaml(
   std::string rawValueString = propertyValueNode.as();
   if (!processor->setProperty(propertyName, rawValueString)) {
 logger_->log_warn(
-"Received property %s with value %s but is not one of the 
properties for %s",
-propertyName.c_str(), rawValueString.c_str(),
-processor->getName().c_str());
+"Received property %s with value %s but it is not one of the 
properties for %s",
+propertyName,
+rawValueString,
+processor->getName());
   }
 }
   }
 }
 
+std::string YamlConfiguration::getOrGenerateId(
+YAML::Node *yamlNode,
+std::string idField) {
+  std::string id;
+  YAML::Node node = yamlNode->as();
+
+  if (node[idField]) {
+if (YAML::NodeType::Scalar == node[idField].Type()) {
+  id = node[idField].as();
+} else {
+  throw std::invalid_argument(
+  "getOrGenerateId: idField is expected to reference YAML::Node "
+  "of YAML::NodeType::Scalar.");
+}
+  } else {
+uuid_t uuid;
+uuid_generate(uuid);
+char uuid_str[37];
+uuid_unparse(uuid, uuid_str);
+id = uuid_str;
+logger_->log_debug("Generating random ID: id => [%s]", id);
+  }
+  return id;
+}
+
+void YamlConfiguration::checkRequiredField(
--- End diff --

Why/how do we need this? operator[] or YamlNode throws an InvalidNode. Do 
you catch this? I did not see that. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] nifi-minifi-cpp pull request #85: MINIFI-275 Bugfix for YAML Configs without...

2017-05-02 Thread phrocker
Github user phrocker commented on a diff in the pull request:

https://github.com/apache/nifi-minifi-cpp/pull/85#discussion_r114138730
  
--- Diff: libminifi/include/core/yaml/YamlConfiguration.h ---
@@ -88,10 +102,17 @@ class YamlConfiguration : public FlowConfiguration {
   void parseRemoteProcessGroupYaml(YAML::Node *node,
core::ProcessGroup * parent);
   // Process Provenance Report YAML
-  void parseProvenanceReportingYaml(YAML::Node *reportNode, 
core::ProcessGroup * parentGroup);
+  void parseProvenanceReportingYaml(YAML::Node *reportNode,
+core::ProcessGroup * parentGroup);
   // Parse Properties Node YAML for a processor
   void parsePropertiesNodeYaml(YAML::Node *propertiesNode,
std::shared_ptr processor);
+  // Helper function to get or generate a component's id field
--- End diff --

Can you use a more verbose comment block here? We're moving away from 
single line comments here. JavaDoc is better and will have a more positive 
impact when we generate docs than these single line function comments. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


<    1   2   3   4   5   6   7   8   9   10   >