[GitHub] nifi-minifi-cpp pull request #99: MINIFI-270: Move Base RPG capabilities bac...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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
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...
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...
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...
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 ...
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...
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...
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...
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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...
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...
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...
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...
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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...
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...
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...
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...
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...
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...
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...
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...
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
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...
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
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
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...
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 ...
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 ...
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 ...
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 ...
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
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...
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.
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.
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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. ---