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.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
