[
https://issues.apache.org/jira/browse/MINIFI-275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15993290#comment-15993290
]
ASF GitHub Bot commented on MINIFI-275:
---------------------------------------
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/85#discussion_r114371075
--- Diff: libminifi/src/core/yaml/YamlConfiguration.cpp ---
@@ -538,14 +561,62 @@ void YamlConfiguration::parsePropertiesNodeYaml(
std::string rawValueString = propertyValueNode.as<std::string>();
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<YAML::Node>();
+
+ if (node[idField]) {
+ if (YAML::NodeType::Scalar == node[idField].Type()) {
+ id = node[idField].as<std::string>();
+ } 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 --
Hey @phrocker,
If I follow you correctly, the concern is about the conditional on line 610?
operator[] returns a bool for the version of yaml-cpp we are using:
https://github.com/jbeder/yaml-cpp/wiki/Tutorial
In older versions of the yaml-cpp api, it did in fact throw an exception as
you describe:
https://github.com/jbeder/yaml-cpp/wiki/How-To-Parse-A-Document-(Old-API)#optional-keys
As to the function in general, it puts the logic in one place for
generating helpful error messages and eliminated the need for if/else or
try/catch blocks throughout the code. The idea is you would call this prior to
using a required field; if the field is not present, a runtime error with a
useful user-facing error message is generated, otherwise you are safe to use
the field without a try/catch or if/else after calling checkRequiredField(...)
Does that answer your question?
I would like input on the function interface (aside from the types, which I
am changing to const std::string &str).. I ended up tacking on a couple
optional fields solely for the use in the error message generation, but it does
seem like a lot to ask the caller just for an assertion check. Let me know if
you see a more elegant way to achieve the desired functionality here.
> Configuration without IDs for components causes exceptions
> ----------------------------------------------------------
>
> Key: MINIFI-275
> URL: https://issues.apache.org/jira/browse/MINIFI-275
> Project: Apache NiFi MiNiFi
> Issue Type: Bug
> Components: C++, Processing Configuration
> Reporter: Aldrin Piri
> Assignee: Kevin Doran
> Priority: Blocker
> Fix For: cpp-0.2.0
>
> Attachments: config.TEST.yml
>
>
> One of the changes to how components are handled in C++ introduced a defect
> into the original construct over the version 1 schema of the YAML.
> The absence of this ID causes a YAML exception.
> We should provide handling to support configurations how they were created
> originally, possibly providing a default/generated ID where one isn't
> specified, and start laying the foundation for versioned schemas as provided
> in our Java implementation.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
