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 2022/05/16 11:40:56 UTC
[GitHub] [nifi-minifi-cpp] fgerlits opened a new pull request, #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
fgerlits opened a new pull request, #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334
https://issues.apache.org/jira/browse/MINIFICPP-1803
Processor and ControllerService properties, relationships and other attributes are now static, and can be read without instantiating the class.
There are 4 cases, represented by the new `ResourceType` enum:
* `Processor`: a subclass of `Processor`, used to be registered using `REGISTER_RESOURCE[_AS]`
* `ControllerService`: a subclass of `ControllerService`, used to be registered using `REGISTER_RESOURCE[_AS]`
* `InternalResource`: a subclass of either `Processor` or `ControllerService`, used to be registered using `REGISTER_INTERNAL_RESOUCE[_AS]`
* `DescriptionOnly`: used to be registered using `REGISTER_RESOURCE[_AS]`, but not a subclass of either `Processor` or `ControllerService`.
Ideally, `properties()` and `relationships()` should be `constexpr` variables instead of functions, too. This will be done later as part of https://issues.apache.org/jira/browse/MINIFICPP-1825.
The manifest has mostly stayed identical, with these differences:
* minifi-coap:
* CoapConnectorService is no longer included in the manifest. Since this controller service is marked as 'internal resource', I think this is correct.
* minifi-jni:
* ExecuteJavaProcessor was not included in the manifest, but now it is. I don't know why it wasn't included earlier.
* The descriptions of ExecuteJavaControllerService and JavaControllerService are slightly different: I have deleted a space from the end.
* minifi-mqtt:
* ConvertHeartbeat, ConvertJSONAck, ConvertUpdate used to be included in the manifest, and they aren't any longer. Since these processors are marked as 'internal resource', I think this is correct.
* minifi-script-extensions:
* The ReloadOnScriptChange property was not included in the ExecutePythonProcessor section of the manifest. This was a bug, fixed now.
* minifi-sensors:
* This extension was missing from the manifest on main, not sure why.
* minifi-standard-processors:
* The description of ExecuteProcess is slightly different: I have added 2 missing spaces.
* minifi-system:
* The Battery Status Discharge property of LinuxPowerManagerService was missing from the old manifest. This was a bug, fixed now.
* The description of NetworkPrioritizerService is slightly diferent: fixed typo ouput -> output.
* minifi-test-processors:
* The description of KamikazeProcessor is slightly diferent: fixed typo configration -> configuration.
* Not sure if we should include this artifact in the manifest, but we did, so we still do.
---
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)?
- [ ] 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] fgerlits commented on a diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885772276
##########
extensions/azure/processors/PutAzureBlobStorage.cpp:
##########
@@ -22,41 +22,14 @@
#include "core/ProcessContext.h"
#include "core/ProcessSession.h"
+#include "core/PropertyBuilder.h"
Review Comment:
I left these in on purpose, as I thought I'd move the static definitions back from the `...StaticDefinitions.cpp` files soon. Since it looks like they will stay like that for a while, I have removed these includes in 416f85452eb9a640de42bc52c9619d459bc5e5cf
--
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 a diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
martinzink commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r896555840
##########
extensions/script/ExecuteScript.h:
##########
@@ -137,13 +137,35 @@ class ExecuteScript : public core::Processor {
engine_factory_(Success, Failure, logger_) {
}
- static core::Property ScriptEngine;
- static core::Property ScriptFile;
- static core::Property ScriptBody;
- static core::Property ModuleDirectory;
+ EXTENSIONAPI static constexpr const char* Description = "Executes a script given the flow file and a process session. "
+ "The script is responsible for handling the incoming flow file (transfer to SUCCESS or remove, e.g.) as well as "
+ "any flow files created by the script. If the handling is incomplete or incorrect, the session will be rolled back.Scripts must define an onTrigger function which accepts NiFi Context "
+ "and Property objects. For efficiency, scripts are executed once when the processor is run, then the onTrigger method is called for each incoming flowfile. This enables scripts to keep state "
+ "if they wish, although there will be a script context per concurrent task of the processor. In order to, e.g., compute an arithmetic sum based on incoming flow file information, set the "
+ "concurrent tasks to 1.";
+
+ EXTENSIONAPI static core::Property ScriptEngine;
+ EXTENSIONAPI static core::Property ScriptFile;
+ EXTENSIONAPI static core::Property ScriptBody;
Review Comment:
I noticed you fixed the constness for the properties you touched. Kudos for that. :+1:
I think this could be made const aswell.
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885582032
##########
extensions/aws/controllerservices/AWSCredentialsService.h:
##########
@@ -53,6 +48,25 @@ class AWSCredentialsService : public core::controller::ControllerService {
: ControllerService(name) {
}
+ EXTENSIONAPI static constexpr const char* Description = "AWS Credentials Management Service";
+
+ EXTENSIONAPI static const core::Property UseDefaultCredentials;
+ EXTENSIONAPI static const core::Property AccessKey;
+ EXTENSIONAPI static const core::Property SecretKey;
+ EXTENSIONAPI static const core::Property CredentialsFile;
+ static auto properties() {
+ return std::array{
+ UseDefaultCredentials,
+ AccessKey,
+ SecretKey,
+ CredentialsFile
+ };
+ }
+
+ EXTENSIONAPI static constexpr bool SupportsDynamicProperties = false;
+ EXTENSIONAPI static constexpr bool SupportsDynamicRelationships = false;
Review Comment:
Yeah, it didn't make much sense, but I didn't want to change the existing logic.
Currently, every controller service in the manifest has a `"supportsDynamicRelationships": false` line; should I remove that, or leave it hard-coded to false?
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r888187635
##########
run_clang_tidy.sh:
##########
@@ -5,7 +5,14 @@ set -uo pipefail
exit_code=0
FILES=$1
+EXCLUDED_EXTENSIONS=("pdh", "windows-event-log", "tensorflow")
+
for changed_file in ${FILES}; do
+ for extension in "${EXCLUDED_EXTENSIONS[@]}"; do
+ if [[ "${changed_file}" =~ extensions/${extension}/ ]]; then
+ continue 2
+ fi
+ done
if [[ "${changed_file}" == *.cpp ]]; then
clang-tidy-14 -warnings-as-errors=* -quiet -p build "${changed_file}"
Review Comment:
I have added this as a quick-and dirty way of skipping the Windows-only extensions. @lordgamez please let me know if you can think of a better way.
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r886761331
##########
libminifi/include/controllers/keyvalue/AbstractAutoPersistingKeyValueStoreService.h:
##########
@@ -30,23 +29,17 @@
#include "core/logging/LoggerConfiguration.h"
#include "utils/Export.h"
-
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-namespace controllers {
+namespace org::apache::nifi::minifi::controllers {
class AbstractAutoPersistingKeyValueStoreService : virtual public PersistableKeyValueStoreService {
public:
explicit AbstractAutoPersistingKeyValueStoreService(const std::string& name, const utils::Identifier& uuid = {});
~AbstractAutoPersistingKeyValueStoreService() override;
- MINIFIAPI static core::Property AlwaysPersist;
- MINIFIAPI static core::Property AutoPersistenceInterval;
+ static constexpr const char* AlwaysPersistPropertyName = "Always Persist";
+ static constexpr const char* AutoPersistenceIntervalPropertyName = "Auto Persistence Interval";
Review Comment:
what is the reason behind this change?
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r886771529
##########
libminifi/src/core/Connectable.cpp:
##########
@@ -85,7 +69,7 @@ bool Connectable::isSupportedRelationship(const core::Relationship &relationship
}
}
-bool Connectable::setAutoTerminatedRelationships(const std::set<Relationship> &relationships) {
+bool Connectable::setAutoTerminatedRelationships(const std::vector<Relationship>& relationships) {
Review Comment:
shouldn't we turn this into a template as well? (as `setSupportedProperties`)
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885559871
##########
extensions/http-curl/protocols/RESTSender.cpp:
##########
@@ -172,10 +167,6 @@ C2Payload RESTSender::sendPayload(const std::string url, const Direction directi
}
}
-REGISTER_RESOURCE(RESTSender, "Encapsulates the restful protocol that is built upon C2Protocol.");
+REGISTER_RESOURCE(RESTSender, DescriptionOnly);
Review Comment:
Let's leave it as it is for now; we can revisit this and clean it up later.
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r886814428
##########
libminifi/include/controllers/keyvalue/AbstractAutoPersistingKeyValueStoreService.h:
##########
@@ -30,23 +29,17 @@
#include "core/logging/LoggerConfiguration.h"
#include "utils/Export.h"
-
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-namespace controllers {
+namespace org::apache::nifi::minifi::controllers {
class AbstractAutoPersistingKeyValueStoreService : virtual public PersistableKeyValueStoreService {
public:
explicit AbstractAutoPersistingKeyValueStoreService(const std::string& name, const utils::Identifier& uuid = {});
~AbstractAutoPersistingKeyValueStoreService() override;
- MINIFIAPI static core::Property AlwaysPersist;
- MINIFIAPI static core::Property AutoPersistenceInterval;
+ static constexpr const char* AlwaysPersistPropertyName = "Always Persist";
+ static constexpr const char* AutoPersistenceIntervalPropertyName = "Auto Persistence Interval";
Review Comment:
This class has two derived classes, in two different libraries: `UnorderedMapPersistableKeyValueStoreService` in `libminifi` and `RocksDbPersistableKeyValueStoreService` in `rocksdb-repos`. The static definitions of each inheritance hierarchy need to be in a single translation unit to ensure the correct order of initialization at startup, but that is not possible if they are in different libraries. So instead of the two derived classes inheriting these two properties, they define them separately, but they both use these constants to indicate that they are really the same properties.
--
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] szaszm commented on a diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885502125
##########
extensions/http-curl/protocols/RESTSender.cpp:
##########
@@ -172,10 +167,6 @@ C2Payload RESTSender::sendPayload(const std::string url, const Direction directi
}
}
-REGISTER_RESOURCE(RESTSender, "Encapsulates the restful protocol that is built upon C2Protocol.");
+REGISTER_RESOURCE(RESTSender, DescriptionOnly);
Review Comment:
I didn't realize there is a third variable, whether it's included in the manifest. With that in mind, I wouldn't do complete autodetection, but rather keep the explicit declaration of "internalness". Adding an "isInternal" member could work, but leaving it as it is is fine, too.
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885582032
##########
extensions/aws/controllerservices/AWSCredentialsService.h:
##########
@@ -53,6 +48,25 @@ class AWSCredentialsService : public core::controller::ControllerService {
: ControllerService(name) {
}
+ EXTENSIONAPI static constexpr const char* Description = "AWS Credentials Management Service";
+
+ EXTENSIONAPI static const core::Property UseDefaultCredentials;
+ EXTENSIONAPI static const core::Property AccessKey;
+ EXTENSIONAPI static const core::Property SecretKey;
+ EXTENSIONAPI static const core::Property CredentialsFile;
+ static auto properties() {
+ return std::array{
+ UseDefaultCredentials,
+ AccessKey,
+ SecretKey,
+ CredentialsFile
+ };
+ }
+
+ EXTENSIONAPI static constexpr bool SupportsDynamicProperties = false;
+ EXTENSIONAPI static constexpr bool SupportsDynamicRelationships = false;
Review Comment:
Yeah, it didn't make much sense, but I didn't want to change the existing logic.
Currently, every controller service in the manifest has a `"supportsDynamicRelationships": false` line; should I remove that, or leave it hard-coded to false?
--
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] szaszm commented on a diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885534872
##########
libminifi/include/agent/agent_docs.h:
##########
@@ -13,53 +13,117 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-#ifndef LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-#define LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-
-#include <stdlib.h>
-#include <utils/StringUtils.h>
+#pragma once
#include <map>
#include <string>
#include <utility>
+#include <vector>
+
+#include "core/Annotation.h"
+#include "core/Property.h"
+#include "core/Relationship.h"
+#include "utils/Export.h"
+#include "utils/StringUtils.h"
+
+namespace org::apache::nifi::minifi {
+
+enum class ResourceType {
+ Processor, ControllerService, InternalResource, DescriptionOnly
+};
+
+struct ClassDescription {
+ ResourceType type_ = ResourceType::Processor;
+ std::string short_name_{};
+ std::string full_name_{};
+ std::string description_{};
+ std::vector<core::Property> class_properties_{};
+ std::vector<core::Relationship> class_relationships_{};
+ bool dynamic_properties_ = false;
+ bool dynamic_relationships_ = false;
+ std::string inputRequirement_{};
+ bool isSingleThreaded_ = false;
+};
+
+struct Components {
+ std::vector<ClassDescription> processors_;
+ std::vector<ClassDescription> controller_services_;
+ std::vector<ClassDescription> other_components_;
+
+ [[nodiscard]] bool empty() const noexcept {
+ return processors_.empty() && controller_services_.empty() && other_components_.empty();
+ }
+};
+
+namespace detail {
+template<typename Container>
+auto toVector(const Container& container) {
+ return std::vector<typename Container::value_type>(container.begin(), container.end());
+}
+
+template<typename T>
+std::string classNameWithDots() {
+ std::string class_name = core::getClassName<T>();
+ return utils::StringUtils::replaceAll(class_name, "::", ".");
+}
+} // namespace detail
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
class AgentDocs {
private:
- static std::map<std::string, std::string> &getDescriptions();
+ MINIFIAPI static std::map<std::string, Components> class_mappings_;
public:
- /**
- * Updates the internal map with the feature description
- * @param feature feature ( CS or processor ) whose description is being provided.
- * @param description provided description.
- * @return true if update occurred.
- */
- static bool putDescription(const std::string &feature, const std::string &description) {
- return getDescriptions().insert(std::make_pair(feature, description)).second;
+ static const std::map<std::string, Components>& getClassDescriptions() {
+ return class_mappings_;
}
- /**
- * Gets the description for the provided feature.
- * @param feature feature whose description we will provide
- * @return true if found, false otherwise
- */
- static bool getDescription(const std::string &feature, std::string &value) {
- const std::map<std::string, std::string> &extensions = getDescriptions();
- auto iff = extensions.find(feature);
- if (iff != extensions.end()) {
- value = iff->second;
- return true;
- } else {
- return false;
+ static bool getDescription(const std::string &feature, std::string &value);
+
+ template<typename Class, ResourceType Type>
Review Comment:
Given the other discussion, this is kind of obsolete now.
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885780882
##########
extensions/aws/controllerservices/AWSCredentialsService.h:
##########
@@ -53,6 +48,25 @@ class AWSCredentialsService : public core::controller::ControllerService {
: ControllerService(name) {
}
+ EXTENSIONAPI static constexpr const char* Description = "AWS Credentials Management Service";
+
+ EXTENSIONAPI static const core::Property UseDefaultCredentials;
+ EXTENSIONAPI static const core::Property AccessKey;
+ EXTENSIONAPI static const core::Property SecretKey;
+ EXTENSIONAPI static const core::Property CredentialsFile;
+ static auto properties() {
+ return std::array{
+ UseDefaultCredentials,
+ AccessKey,
+ SecretKey,
+ CredentialsFile
+ };
+ }
+
+ EXTENSIONAPI static constexpr bool SupportsDynamicProperties = false;
+ EXTENSIONAPI static constexpr bool SupportsDynamicRelationships = false;
Review Comment:
Done in 3c2574ae73f8020181503d669161607601f861cf
The manifest will still have a `"supportsDynamicRelationships": false` line in each controller service block, as it did before this PR, I hope that's OK. We can remove it later, after checking with C2 developers.
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r887981141
##########
run_clang_tidy.sh:
##########
@@ -6,6 +6,9 @@ exit_code=0
FILES=$1
for changed_file in ${FILES}; do
+ if [[ "${changed_file}" =~ /pdh/ || "${changed_file}" =~ /windows-event-log/ ]]; then
+ continue
+ fi
if [[ "${changed_file}" == *.cpp ]]; then
clang-tidy-14 -warnings-as-errors=* -quiet -p build "${changed_file}"
Review Comment:
I have added this as a quick-and dirty way of skipping the Windows-only extensions. @lordgamez please let me know if you can think of a better way.
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885772276
##########
extensions/azure/processors/PutAzureBlobStorage.cpp:
##########
@@ -22,41 +22,14 @@
#include "core/ProcessContext.h"
#include "core/ProcessSession.h"
+#include "core/PropertyBuilder.h"
Review Comment:
I left these in on purpose, as I thought I'd move the static definitions back from the `...StaticDefinitions.cpp` soon. Since it looks like they will stay like that for a while, I have removed these includes in 416f85452eb9a640de42bc52c9619d459bc5e5cf
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r896467745
##########
extensions/standard-processors/processors/ExtractText.h:
##########
@@ -36,26 +36,43 @@ namespace minifi {
namespace processors {
//! ExtractText Class
-class ExtractText : public core::Processor {
+class
+ExtractText : public core::Processor {
Review Comment:
yes it was, thanks for catching it -- fixed in f4f8c9a11f2fd379c3b5593441619ad547736246
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r890872361
##########
libminifi/src/core/Connectable.cpp:
##########
@@ -85,7 +69,7 @@ bool Connectable::isSupportedRelationship(const core::Relationship &relationship
}
}
-bool Connectable::setAutoTerminatedRelationships(const std::set<Relationship> &relationships) {
+bool Connectable::setAutoTerminatedRelationships(const std::vector<Relationship>& relationships) {
Review Comment:
I prefer `some_function(std::array{foo, bar})` to `some_function({foo, bar})`; I think it's clearer and it's also slightly more efficient. The only downside is a few more characters, but I don't think that's a problem.
--
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] szaszm closed pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
szaszm closed pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885773328
##########
extensions/http-curl/processors/InvokeHTTP.cpp:
##########
@@ -121,57 +121,36 @@ core::Property InvokeHTTP::InvalidHTTPHeaderFieldHandlingStrategy(
->withAllowableValues<std::string>(InvalidHTTPHeaderFieldHandlingOption::values())
->build());
-const char* InvokeHTTP::STATUS_CODE = "invokehttp.status.code";
-const char* InvokeHTTP::STATUS_MESSAGE = "invokehttp.status.message";
-const char* InvokeHTTP::RESPONSE_BODY = "invokehttp.response.body";
-const char* InvokeHTTP::REQUEST_URL = "invokehttp.request.url";
-const char* InvokeHTTP::TRANSACTION_ID = "invokehttp.tx.id";
-const char* InvokeHTTP::REMOTE_DN = "invokehttp.remote.dn";
-const char* InvokeHTTP::EXCEPTION_CLASS = "invokehttp.java.exception.class";
-const char* InvokeHTTP::EXCEPTION_MESSAGE = "invokehttp.java.exception.message";
-core::Relationship InvokeHTTP::Success("success", "The original FlowFile will be routed upon success (2xx status codes). "
+const core::Relationship InvokeHTTP::Success("success", "The original FlowFile will be routed upon success (2xx status codes). "
"It will have new attributes detailing the success of the request.");
-core::Relationship InvokeHTTP::RelResponse("response", "A Response FlowFile will be routed upon success (2xx status codes). "
+const core::Relationship InvokeHTTP::RelResponse("response", "A Response FlowFile will be routed upon success (2xx status codes). "
"If the 'Always Output Response' property is true then the response will be sent "
"to this relationship regardless of the status code received.");
-core::Relationship InvokeHTTP::RelRetry("retry", "The original FlowFile will be routed on any status code that can be retried "
+const core::Relationship InvokeHTTP::RelRetry("retry", "The original FlowFile will be routed on any status code that can be retried "
"(5xx status codes). It will have new attributes detailing the request.");
-core::Relationship InvokeHTTP::RelNoRetry("no retry", "The original FlowFile will be routed on any status code that should NOT "
+const core::Relationship InvokeHTTP::RelNoRetry("no retry", "The original FlowFile will be routed on any status code that should NOT "
"be retried (1xx, 3xx, 4xx status codes). It will have new attributes detailing the request.");
-core::Relationship InvokeHTTP::RelFailure("failure", "The original FlowFile will be routed on any type of connection failure, "
+const core::Relationship InvokeHTTP::RelFailure("failure", "The original FlowFile will be routed on any type of connection failure, "
"timeout or general exception. It will have new attributes detailing the request.");
+const char* InvokeHTTP::STATUS_CODE = "invokehttp.status.code";
+const char* InvokeHTTP::STATUS_MESSAGE = "invokehttp.status.message";
+const char* InvokeHTTP::RESPONSE_BODY = "invokehttp.response.body";
+const char* InvokeHTTP::REQUEST_URL = "invokehttp.request.url";
+const char* InvokeHTTP::TRANSACTION_ID = "invokehttp.tx.id";
+const char* InvokeHTTP::REMOTE_DN = "invokehttp.remote.dn";
+const char* InvokeHTTP::EXCEPTION_CLASS = "invokehttp.java.exception.class";
+const char* InvokeHTTP::EXCEPTION_MESSAGE = "invokehttp.java.exception.message";
+
Review Comment:
done in a1236cb6b71c1533829b978fc8b309fa5fd40d3c
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r887841392
##########
libminifi/src/core/Connectable.cpp:
##########
@@ -85,7 +69,7 @@ bool Connectable::isSupportedRelationship(const core::Relationship &relationship
}
}
-bool Connectable::setAutoTerminatedRelationships(const std::set<Relationship> &relationships) {
+bool Connectable::setAutoTerminatedRelationships(const std::vector<Relationship>& relationships) {
Review Comment:
I changed `setAutoTerminatedRelationships` to a template in 529f7014bdaf548ebfd5198cd78cdd5ff9b5b136
but then I realized it doesn't need to be a template, and changed `setSupportedProperties`, `setSupportedRelationships` and `setAutoTerminatedRelationships` back to functions, with a `gsl::span` argument type, in cb2e0defeb0d19539590fe82cd6eb69e466ca2dc
let me know if you don't like cb2e0defeb0d19539590fe82cd6eb69e466ca2dc and want it changed back
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885777510
##########
libminifi/include/agent/agent_docs.h:
##########
@@ -13,53 +13,117 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-#ifndef LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-#define LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-
-#include <stdlib.h>
-#include <utils/StringUtils.h>
+#pragma once
#include <map>
#include <string>
#include <utility>
+#include <vector>
+
+#include "core/Annotation.h"
+#include "core/Property.h"
+#include "core/Relationship.h"
+#include "utils/Export.h"
+#include "utils/StringUtils.h"
+
+namespace org::apache::nifi::minifi {
+
+enum class ResourceType {
+ Processor, ControllerService, InternalResource, DescriptionOnly
+};
+
+struct ClassDescription {
+ ResourceType type_ = ResourceType::Processor;
+ std::string short_name_{};
+ std::string full_name_{};
+ std::string description_{};
+ std::vector<core::Property> class_properties_{};
+ std::vector<core::Relationship> class_relationships_{};
+ bool dynamic_properties_ = false;
+ bool dynamic_relationships_ = false;
+ std::string inputRequirement_{};
+ bool isSingleThreaded_ = false;
+};
+
+struct Components {
+ std::vector<ClassDescription> processors_;
+ std::vector<ClassDescription> controller_services_;
+ std::vector<ClassDescription> other_components_;
+
+ [[nodiscard]] bool empty() const noexcept {
+ return processors_.empty() && controller_services_.empty() && other_components_.empty();
+ }
+};
+
+namespace detail {
+template<typename Container>
+auto toVector(const Container& container) {
+ return std::vector<typename Container::value_type>(container.begin(), container.end());
+}
+
+template<typename T>
+std::string classNameWithDots() {
+ std::string class_name = core::getClassName<T>();
+ return utils::StringUtils::replaceAll(class_name, "::", ".");
+}
+} // namespace detail
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
class AgentDocs {
private:
- static std::map<std::string, std::string> &getDescriptions();
+ MINIFIAPI static std::map<std::string, Components> class_mappings_;
public:
- /**
- * Updates the internal map with the feature description
- * @param feature feature ( CS or processor ) whose description is being provided.
- * @param description provided description.
- * @return true if update occurred.
- */
- static bool putDescription(const std::string &feature, const std::string &description) {
- return getDescriptions().insert(std::make_pair(feature, description)).second;
+ static const std::map<std::string, Components>& getClassDescriptions() {
+ return class_mappings_;
}
- /**
- * Gets the description for the provided feature.
- * @param feature feature whose description we will provide
- * @return true if found, false otherwise
- */
- static bool getDescription(const std::string &feature, std::string &value) {
- const std::map<std::string, std::string> &extensions = getDescriptions();
- auto iff = extensions.find(feature);
- if (iff != extensions.end()) {
- value = iff->second;
- return true;
- } else {
- return false;
+ static bool getDescription(const std::string &feature, std::string &value);
+
+ template<typename Class, ResourceType Type>
+ static void createClassDescription(const std::string& group, const std::string& name) {
+ Components& components = class_mappings_[group];
+
+ if constexpr (Type == ResourceType::Processor) {
+ components.processors_.push_back(ClassDescription{
+ .type_ = Type,
+ .short_name_ = name,
+ .full_name_ = detail::classNameWithDots<Class>(),
+ .description_ = Class::Description,
+ .class_properties_ = detail::toVector(Class::properties()),
+ .class_relationships_ = detail::toVector(Class::relationships()),
Review Comment:
I changed this and then then changed it back, because `utils::span_to<std::vector>(...)` did not compile.
`utils::span_to<std::vector<core::Property>>(...)` did compile, but it's quite long, and I did not like the fact that I have to write "property" (or "relationship") twice.
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r887923878
##########
libminifi/src/core/Connectable.cpp:
##########
@@ -85,7 +69,7 @@ bool Connectable::isSupportedRelationship(const core::Relationship &relationship
}
}
-bool Connectable::setAutoTerminatedRelationships(const std::set<Relationship> &relationships) {
+bool Connectable::setAutoTerminatedRelationships(const std::vector<Relationship>& relationships) {
Review Comment:
Unfortunately, initializer lists don't convert implicitly to `gsl::span`. Nevertheless, we used to create a `std::vector` (with elements on the heap) and now we only create a `std::array` (with elements on the stack).
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r887981141
##########
run_clang_tidy.sh:
##########
@@ -6,6 +6,9 @@ exit_code=0
FILES=$1
for changed_file in ${FILES}; do
+ if [[ "${changed_file}" =~ /pdh/ || "${changed_file}" =~ /windows-event-log/ ]]; then
+ continue
+ fi
if [[ "${changed_file}" == *.cpp ]]; then
clang-tidy-14 -warnings-as-errors=* -quiet -p build "${changed_file}"
Review Comment:
I have added this as a quick-and dirty way of skipping the Windows-only extensions. @lordgamez please let me know if you can think of a better way.
--
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] szaszm commented on a diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r888781837
##########
libminifi/src/core/Connectable.cpp:
##########
@@ -85,7 +69,7 @@ bool Connectable::isSupportedRelationship(const core::Relationship &relationship
}
}
-bool Connectable::setAutoTerminatedRelationships(const std::set<Relationship> &relationships) {
+bool Connectable::setAutoTerminatedRelationships(const std::vector<Relationship>& relationships) {
Review Comment:
You can overload them to accept both gsl::span and std::array, if you want.
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r896467745
##########
extensions/standard-processors/processors/ExtractText.h:
##########
@@ -36,26 +36,43 @@ namespace minifi {
namespace processors {
//! ExtractText Class
-class ExtractText : public core::Processor {
+class
+ExtractText : public core::Processor {
Review Comment:
yes it was, thanks for catching it -- fixed in f955513b8c0a1d56546e9aeec7172968609c63a8
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r885490057
##########
extensions/http-curl/protocols/RESTSender.cpp:
##########
@@ -172,10 +167,6 @@ C2Payload RESTSender::sendPayload(const std::string url, const Direction directi
}
}
-REGISTER_RESOURCE(RESTSender, "Encapsulates the restful protocol that is built upon C2Protocol.");
+REGISTER_RESOURCE(RESTSender, DescriptionOnly);
Review Comment:
This is a bit of a mess, but it was already like that before this PR:
- `InternalResource` means it inherits from either `Processor` or `ControllerService`, but should not be included in the manifest. These do not have a `Description` field.
- `DescriptionOnly` means it does not inherit from `Processor` or `ControllerService`, so it will not be included in the manifest, but it does have a `Description` field (which I don't think we use anywhere, but I'm not sure, so I did not want to remove it).
If these `Description`s are in fact not needed, then we could simplify this to:
- if it has a `Description`, then include it in the manifest, detect whether it is a `Processor` or `ControllerService`, and throw a compile error if it's neither;
- if it doesn't have a `Description`, then only register it in the class loader, and skip it in `agent_docs.h` etc completely.
Should we do this?
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r887874442
##########
libminifi/src/core/Connectable.cpp:
##########
@@ -85,7 +69,7 @@ bool Connectable::isSupportedRelationship(const core::Relationship &relationship
}
}
-bool Connectable::setAutoTerminatedRelationships(const std::set<Relationship> &relationships) {
+bool Connectable::setAutoTerminatedRelationships(const std::vector<Relationship>& relationships) {
Review Comment:
I'm fine with `gsl::span` my only concern is from the caller's side we injected a number of `std::array` calls where previously we used an initializer list, do we need them or can we drop them now that the methods are not templates?
--
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] szaszm commented on a diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r881901802
##########
extensions/azure/processors/PutAzureBlobStorage.cpp:
##########
@@ -22,41 +22,14 @@
#include "core/ProcessContext.h"
#include "core/ProcessSession.h"
+#include "core/PropertyBuilder.h"
Review Comment:
This include was missing in the old version, but it's no longer used in this source file.
##########
extensions/aws/controllerservices/AWSCredentialsService.h:
##########
@@ -53,6 +48,25 @@ class AWSCredentialsService : public core::controller::ControllerService {
: ControllerService(name) {
}
+ EXTENSIONAPI static constexpr const char* Description = "AWS Credentials Management Service";
+
+ EXTENSIONAPI static const core::Property UseDefaultCredentials;
+ EXTENSIONAPI static const core::Property AccessKey;
+ EXTENSIONAPI static const core::Property SecretKey;
+ EXTENSIONAPI static const core::Property CredentialsFile;
+ static auto properties() {
+ return std::array{
+ UseDefaultCredentials,
+ AccessKey,
+ SecretKey,
+ CredentialsFile
+ };
+ }
+
+ EXTENSIONAPI static constexpr bool SupportsDynamicProperties = false;
+ EXTENSIONAPI static constexpr bool SupportsDynamicRelationships = false;
Review Comment:
There should be no need for `SupportsDynamicRelationships` for anything other than Processors. Services can't have relationships.
##########
extensions/http-curl/protocols/RESTSender.cpp:
##########
@@ -172,10 +167,6 @@ C2Payload RESTSender::sendPayload(const std::string url, const Direction directi
}
}
-REGISTER_RESOURCE(RESTSender, "Encapsulates the restful protocol that is built upon C2Protocol.");
+REGISTER_RESOURCE(RESTSender, DescriptionOnly);
Review Comment:
I would assume that this is an `InternalResource`. That makes it different?
##########
extensions/http-curl/processors/InvokeHTTP.cpp:
##########
@@ -121,57 +121,36 @@ core::Property InvokeHTTP::InvalidHTTPHeaderFieldHandlingStrategy(
->withAllowableValues<std::string>(InvalidHTTPHeaderFieldHandlingOption::values())
->build());
-const char* InvokeHTTP::STATUS_CODE = "invokehttp.status.code";
-const char* InvokeHTTP::STATUS_MESSAGE = "invokehttp.status.message";
-const char* InvokeHTTP::RESPONSE_BODY = "invokehttp.response.body";
-const char* InvokeHTTP::REQUEST_URL = "invokehttp.request.url";
-const char* InvokeHTTP::TRANSACTION_ID = "invokehttp.tx.id";
-const char* InvokeHTTP::REMOTE_DN = "invokehttp.remote.dn";
-const char* InvokeHTTP::EXCEPTION_CLASS = "invokehttp.java.exception.class";
-const char* InvokeHTTP::EXCEPTION_MESSAGE = "invokehttp.java.exception.message";
-core::Relationship InvokeHTTP::Success("success", "The original FlowFile will be routed upon success (2xx status codes). "
+const core::Relationship InvokeHTTP::Success("success", "The original FlowFile will be routed upon success (2xx status codes). "
"It will have new attributes detailing the success of the request.");
-core::Relationship InvokeHTTP::RelResponse("response", "A Response FlowFile will be routed upon success (2xx status codes). "
+const core::Relationship InvokeHTTP::RelResponse("response", "A Response FlowFile will be routed upon success (2xx status codes). "
"If the 'Always Output Response' property is true then the response will be sent "
"to this relationship regardless of the status code received.");
-core::Relationship InvokeHTTP::RelRetry("retry", "The original FlowFile will be routed on any status code that can be retried "
+const core::Relationship InvokeHTTP::RelRetry("retry", "The original FlowFile will be routed on any status code that can be retried "
"(5xx status codes). It will have new attributes detailing the request.");
-core::Relationship InvokeHTTP::RelNoRetry("no retry", "The original FlowFile will be routed on any status code that should NOT "
+const core::Relationship InvokeHTTP::RelNoRetry("no retry", "The original FlowFile will be routed on any status code that should NOT "
"be retried (1xx, 3xx, 4xx status codes). It will have new attributes detailing the request.");
-core::Relationship InvokeHTTP::RelFailure("failure", "The original FlowFile will be routed on any type of connection failure, "
+const core::Relationship InvokeHTTP::RelFailure("failure", "The original FlowFile will be routed on any type of connection failure, "
"timeout or general exception. It will have new attributes detailing the request.");
+const char* InvokeHTTP::STATUS_CODE = "invokehttp.status.code";
+const char* InvokeHTTP::STATUS_MESSAGE = "invokehttp.status.message";
+const char* InvokeHTTP::RESPONSE_BODY = "invokehttp.response.body";
+const char* InvokeHTTP::REQUEST_URL = "invokehttp.request.url";
+const char* InvokeHTTP::TRANSACTION_ID = "invokehttp.tx.id";
+const char* InvokeHTTP::REMOTE_DN = "invokehttp.remote.dn";
+const char* InvokeHTTP::EXCEPTION_CLASS = "invokehttp.java.exception.class";
+const char* InvokeHTTP::EXCEPTION_MESSAGE = "invokehttp.java.exception.message";
+
Review Comment:
Could you make these `const` or `constexpr` as well?
##########
Extensions.md:
##########
@@ -20,14 +20,16 @@ To enable all extensions for your platform, you may use -DENABLE_ALL=TRUE OR sel
Extensions are dynamic libraries loaded at runtime by the agent. An extension makes its
capabilities (classes) available to the system through registrars. Registration must happen in source files, not headers.
-``` C++
+```C++
// register user-facing classes as
-REGISTER_RESOURCE(InvokeHTTP, "An HTTP client processor which can interact with a configurable HTTP Endpoint. "
- "The destination URL and HTTP Method are configurable. FlowFile attributes are converted to HTTP headers and the "
- "FlowFile contents are included as the body of the request (if the HTTP Method is PUT, POST or PATCH).");
+REGISTER_RESOURCE(InvokeHTTP, Processor);
+// or
+REGISTER_RESOURCE(SSLContextService, ControllerService);
Review Comment:
We should be able to determine the type with introspection (`is_base_of`). I think that would be a better API.
##########
libminifi/include/agent/agent_docs.h:
##########
@@ -13,53 +13,117 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-#ifndef LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-#define LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-
-#include <stdlib.h>
-#include <utils/StringUtils.h>
+#pragma once
#include <map>
#include <string>
#include <utility>
+#include <vector>
+
+#include "core/Annotation.h"
+#include "core/Property.h"
+#include "core/Relationship.h"
+#include "utils/Export.h"
+#include "utils/StringUtils.h"
+
+namespace org::apache::nifi::minifi {
+
+enum class ResourceType {
+ Processor, ControllerService, InternalResource, DescriptionOnly
+};
+
+struct ClassDescription {
+ ResourceType type_ = ResourceType::Processor;
+ std::string short_name_{};
+ std::string full_name_{};
+ std::string description_{};
+ std::vector<core::Property> class_properties_{};
+ std::vector<core::Relationship> class_relationships_{};
+ bool dynamic_properties_ = false;
+ bool dynamic_relationships_ = false;
+ std::string inputRequirement_{};
+ bool isSingleThreaded_ = false;
+};
+
+struct Components {
+ std::vector<ClassDescription> processors_;
+ std::vector<ClassDescription> controller_services_;
+ std::vector<ClassDescription> other_components_;
+
+ [[nodiscard]] bool empty() const noexcept {
+ return processors_.empty() && controller_services_.empty() && other_components_.empty();
+ }
+};
+
+namespace detail {
+template<typename Container>
+auto toVector(const Container& container) {
+ return std::vector<typename Container::value_type>(container.begin(), container.end());
+}
+
+template<typename T>
+std::string classNameWithDots() {
+ std::string class_name = core::getClassName<T>();
+ return utils::StringUtils::replaceAll(class_name, "::", ".");
+}
+} // namespace detail
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
class AgentDocs {
private:
- static std::map<std::string, std::string> &getDescriptions();
+ MINIFIAPI static std::map<std::string, Components> class_mappings_;
public:
- /**
- * Updates the internal map with the feature description
- * @param feature feature ( CS or processor ) whose description is being provided.
- * @param description provided description.
- * @return true if update occurred.
- */
- static bool putDescription(const std::string &feature, const std::string &description) {
- return getDescriptions().insert(std::make_pair(feature, description)).second;
+ static const std::map<std::string, Components>& getClassDescriptions() {
+ return class_mappings_;
}
- /**
- * Gets the description for the provided feature.
- * @param feature feature whose description we will provide
- * @return true if found, false otherwise
- */
- static bool getDescription(const std::string &feature, std::string &value) {
- const std::map<std::string, std::string> &extensions = getDescriptions();
- auto iff = extensions.find(feature);
- if (iff != extensions.end()) {
- value = iff->second;
- return true;
- } else {
- return false;
+ static bool getDescription(const std::string &feature, std::string &value);
+
+ template<typename Class, ResourceType Type>
+ static void createClassDescription(const std::string& group, const std::string& name) {
+ Components& components = class_mappings_[group];
+
+ if constexpr (Type == ResourceType::Processor) {
+ components.processors_.push_back(ClassDescription{
+ .type_ = Type,
+ .short_name_ = name,
+ .full_name_ = detail::classNameWithDots<Class>(),
+ .description_ = Class::Description,
+ .class_properties_ = detail::toVector(Class::properties()),
+ .class_relationships_ = detail::toVector(Class::relationships()),
Review Comment:
I wrote a similar utility to `toVector` in `utils/gsl.h`: `span_to`. I think it could be used here like this:
```suggestion
.class_properties_ = utils::span_to<std::vector>(Class::properties()),
.class_relationships_ = utils::span_to<std::vector>(Class::relationships()),
```
The arrays should be implicitly convertible to `gsl::span`. Consider using it if you like it, but if not, that's OK as well.
##########
libminifi/include/agent/agent_docs.h:
##########
@@ -13,53 +13,117 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-#ifndef LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-#define LIBMINIFI_INCLUDE_AGENT_AGENT_DOCS_H_
-
-#include <stdlib.h>
-#include <utils/StringUtils.h>
+#pragma once
#include <map>
#include <string>
#include <utility>
+#include <vector>
+
+#include "core/Annotation.h"
+#include "core/Property.h"
+#include "core/Relationship.h"
+#include "utils/Export.h"
+#include "utils/StringUtils.h"
+
+namespace org::apache::nifi::minifi {
+
+enum class ResourceType {
+ Processor, ControllerService, InternalResource, DescriptionOnly
+};
+
+struct ClassDescription {
+ ResourceType type_ = ResourceType::Processor;
+ std::string short_name_{};
+ std::string full_name_{};
+ std::string description_{};
+ std::vector<core::Property> class_properties_{};
+ std::vector<core::Relationship> class_relationships_{};
+ bool dynamic_properties_ = false;
+ bool dynamic_relationships_ = false;
+ std::string inputRequirement_{};
+ bool isSingleThreaded_ = false;
+};
+
+struct Components {
+ std::vector<ClassDescription> processors_;
+ std::vector<ClassDescription> controller_services_;
+ std::vector<ClassDescription> other_components_;
+
+ [[nodiscard]] bool empty() const noexcept {
+ return processors_.empty() && controller_services_.empty() && other_components_.empty();
+ }
+};
+
+namespace detail {
+template<typename Container>
+auto toVector(const Container& container) {
+ return std::vector<typename Container::value_type>(container.begin(), container.end());
+}
+
+template<typename T>
+std::string classNameWithDots() {
+ std::string class_name = core::getClassName<T>();
+ return utils::StringUtils::replaceAll(class_name, "::", ".");
+}
+} // namespace detail
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
class AgentDocs {
private:
- static std::map<std::string, std::string> &getDescriptions();
+ MINIFIAPI static std::map<std::string, Components> class_mappings_;
public:
- /**
- * Updates the internal map with the feature description
- * @param feature feature ( CS or processor ) whose description is being provided.
- * @param description provided description.
- * @return true if update occurred.
- */
- static bool putDescription(const std::string &feature, const std::string &description) {
- return getDescriptions().insert(std::make_pair(feature, description)).second;
+ static const std::map<std::string, Components>& getClassDescriptions() {
+ return class_mappings_;
}
- /**
- * Gets the description for the provided feature.
- * @param feature feature whose description we will provide
- * @return true if found, false otherwise
- */
- static bool getDescription(const std::string &feature, std::string &value) {
- const std::map<std::string, std::string> &extensions = getDescriptions();
- auto iff = extensions.find(feature);
- if (iff != extensions.end()) {
- value = iff->second;
- return true;
- } else {
- return false;
+ static bool getDescription(const std::string &feature, std::string &value);
+
+ template<typename Class, ResourceType Type>
Review Comment:
The ResourceType could be autodetected IMO. Included some untested detection logic that will probably need some changes, but should at least demonstrate the idea:
```suggestion
namespace detail {
template<typename T>
constexpr bool is_array = false;
template<typename ValueType, size_t Size>
constexpr bool is_array<std::array<ValueType, Size>> = true;
template<typename T, typename ValueType>
concept array_of = is_array<T> && std::is_same_v<typename T::value_type, ValueType>;
template<typename T>
concept has_description = requires() {
{ T::Description } -> const char*;
};
template<typename T>
concept has_metadata = has_description<T> && requires() {
{ T::properties() } -> array_of<core::Property>;
{ T::SupportsDynamicProperties } -> bool;
};
template<typename T>
concept processor = has_description<T> && std::derived_from<T, core::Processor>;
template<typename T>
concept controller_service = has_description<T> && std::derived_from<T, core::controller::ControllerService>;
struct unknown_resource_type : std::exception {};
template<typename T>
constexpr ResourceType resource_type_of() {
if constexpr (processor<T>) {
return ResourceType::Processor;
} else if constexpr (controller_service<T>) {
return ResourceType::ControllerService;
} else if constexpr (has_metadata<T>) {
return ResourceType::InternalResource;
} else if constepxr (has_description<T>) {
return ResourceType::DescriptionOnly;
} else {
throw unknown_resource_type{};
}
}
} // namespace detail
template<typename Class, ResourceType Type>
```
--
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 diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
fgerlits commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r896702212
##########
extensions/script/ExecuteScript.h:
##########
@@ -137,13 +137,35 @@ class ExecuteScript : public core::Processor {
engine_factory_(Success, Failure, logger_) {
}
- static core::Property ScriptEngine;
- static core::Property ScriptFile;
- static core::Property ScriptBody;
- static core::Property ModuleDirectory;
+ EXTENSIONAPI static constexpr const char* Description = "Executes a script given the flow file and a process session. "
+ "The script is responsible for handling the incoming flow file (transfer to SUCCESS or remove, e.g.) as well as "
+ "any flow files created by the script. If the handling is incomplete or incorrect, the session will be rolled back.Scripts must define an onTrigger function which accepts NiFi Context "
+ "and Property objects. For efficiency, scripts are executed once when the processor is run, then the onTrigger method is called for each incoming flowfile. This enables scripts to keep state "
+ "if they wish, although there will be a script context per concurrent task of the processor. In order to, e.g., compute an arithmetic sum based on incoming flow file information, set the "
+ "concurrent tasks to 1.";
+
+ EXTENSIONAPI static core::Property ScriptEngine;
+ EXTENSIONAPI static core::Property ScriptFile;
+ EXTENSIONAPI static core::Property ScriptBody;
Review Comment:
yes, I missed this, thanks -- fixed in 9becda49a60685b0b7f67e4b5678d01e238f44a4
--
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] szaszm commented on a diff in pull request #1334: MINIFICPP-1803 Do not instantiate all classes at heartbeat
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1334:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1334#discussion_r895881828
##########
extensions/standard-processors/processors/ExtractText.h:
##########
@@ -36,26 +36,43 @@ namespace minifi {
namespace processors {
//! ExtractText Class
-class ExtractText : public core::Processor {
+class
+ExtractText : public core::Processor {
Review Comment:
I think this strange formatting was done by mistake.
--
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