You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/10/21 16:37:16 UTC

[GitHub] [nifi-minifi-cpp] fgerlits opened a new pull request #1206: MINIFICPP-1671 Load the extensions before creating the repositories

fgerlits opened a new pull request #1206:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1206


   https://issues.apache.org/jira/browse/MINIFICPP-1671
   
   We need to load the extensions before the repositories are created, otherwise we would always create Volatile repositories instead of the RocksDB ones.
   
   It would be good to have some sort of test to validate this, but I don't know how.  A sort of indirect validation is that #1017 works with this change and fails without it.
   
   ---
   
   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:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced
        in the commit message?
   
   - [x] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [x] Has your PR been rebased against the latest commit within the target branch (typically main)?
   
   - [x] Is your initial contribution a single, squashed commit?
   
   ### For code changes:
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] If applicable, have you updated the LICENSE file?
   - [ ] If applicable, have you updated the NOTICE file?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] martinzink edited a comment on pull request #1206: MINIFICPP-1671 Load the extensions before creating the repositories

Posted by GitBox <gi...@apache.org>.
martinzink edited a comment on pull request #1206:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1206#issuecomment-949595085


   Nice catch :+1: 
   For testing maybe we could log when we fallback to volatile repository, and verify that the fallback log is not present, when we are configured to use RocksDB?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1206: MINIFICPP-1671 Load the extensions before creating the repositories

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1206:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1206#discussion_r734287980



##########
File path: libminifi/src/core/FlowConfiguration.cpp
##########
@@ -60,8 +60,6 @@ FlowConfiguration::FlowConfiguration(
     }
     checksum_calculator_.setFileLocation(*config_path_);
   }
-
-  extension::ExtensionManager::get().initialize(configuration_);
 }

Review comment:
       it should be safe to remove the `#include "core/extension/ExtensionManager.h"`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1206: MINIFICPP-1671 Load the extensions before creating the repositories

Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1206:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1206#discussion_r734574770



##########
File path: libminifi/src/core/FlowConfiguration.cpp
##########
@@ -60,8 +60,6 @@ FlowConfiguration::FlowConfiguration(
     }
     checksum_calculator_.setFileLocation(*config_path_);
   }
-
-  extension::ExtensionManager::get().initialize(configuration_);
 }

Review comment:
       yup, done in https://github.com/apache/nifi-minifi-cpp/pull/1206/commits/c0d39c32e6c0c637ac74f0d6ff258915ce46f7d5




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on pull request #1206: MINIFICPP-1671 Load the extensions before creating the repositories

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on pull request #1206:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1206#issuecomment-949349925


   nice catch!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] martinzink commented on pull request #1206: MINIFICPP-1671 Load the extensions before creating the repositories

Posted by GitBox <gi...@apache.org>.
martinzink commented on pull request #1206:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1206#issuecomment-949595085


   Nice catch
   For testing maybe we could log when we fallback to volatile repository, and verify that the fallback log is not present, when we are configured to use RocksDB?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #1206: MINIFICPP-1671 Load the extensions before creating the repositories

Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #1206:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1206#discussion_r734574770



##########
File path: libminifi/src/core/FlowConfiguration.cpp
##########
@@ -60,8 +60,6 @@ FlowConfiguration::FlowConfiguration(
     }
     checksum_calculator_.setFileLocation(*config_path_);
   }
-
-  extension::ExtensionManager::get().initialize(configuration_);
 }

Review comment:
       yup, done




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] fgerlits commented on pull request #1206: MINIFICPP-1671 Load the extensions before creating the repositories

Posted by GitBox <gi...@apache.org>.
fgerlits commented on pull request #1206:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1206#issuecomment-949667358


   > Nice catch +1 For testing maybe we could log when we fallback to volatile repository, and verify that the fallback log is not present, when we are configured to use RocksDB?
   
   Yeah -- it would be better to check that some functionality of the RocksDB repos (eg. persistence) works, but checking the logs is better than nothing.  Done in https://github.com/apache/nifi-minifi-cpp/pull/1206/commits/bee3524b38acbeb076a87bcc9788d38f151d5950.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni closed pull request #1206: MINIFICPP-1671 Load the extensions before creating the repositories

Posted by GitBox <gi...@apache.org>.
adamdebreceni closed pull request #1206:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1206


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org