[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1220: MINIFICPP-1694 Remove false positive error log messages

[GitBox <gi...@apache.org>]

adamdebreceni commented on a change in pull request #1220:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1220#discussion_r764680221



##########
File path: libminifi/src/core/yaml/YamlConfiguration.cpp
##########
@@ -529,14 +529,20 @@ void YamlConfiguration::parseControllerServices(const YAML::Node& controllerServ
       CONFIG_YAML_CONTROLLER_SERVICES_KEY);
       std::string type = "";
 
-      try {
-        yaml::checkRequiredField(&controllerServiceNode, "class", logger_, CONFIG_YAML_CONTROLLER_SERVICES_KEY);
+      if (yaml::isFieldPresent(&controllerServiceNode, "class")) {
         type = controllerServiceNode["class"].as<std::string>();
-      } catch (const std::invalid_argument &) {
-        yaml::checkRequiredField(&controllerServiceNode, "type", logger_, CONFIG_YAML_CONTROLLER_SERVICES_KEY);
+      } else {
+        const YAML::Node name_node = controllerServiceNode.as<YAML::Node>()["name"];
+        std::string err_msg =
+          name_node ?
+            "Unable to parse configuration file for component named '" + name_node.as<std::string>() + "' as required field 'class' or 'type' is missing" :
+            "Unable to parse configuration file as required field 'class' or 'type' is missing";
+        err_msg += " [in '" + std::string(CONFIG_YAML_CONTROLLER_SERVICES_KEY) + "' section of configuration file]";
+        yaml::checkRequiredField(&controllerServiceNode, "type", logger_, CONFIG_YAML_CONTROLLER_SERVICES_KEY, err_msg);

Review comment:
       it seems we have duplicated the error message from `checkRequiredField`, should we maybe create a `checkRequiredField` overload that takes a list of alternative keys (here {"class", "type"})? we could even add a "parser" parameter, and use it as
   
   ```
   template<typename T>
   T parseRequiredField(const YAML::Node* yamlNode, const std::vector<std::string>& alternateNames, const std::shared_ptr<logging::Logger>& logger, const std::string &yamlSection, std::function<T(const YAML::Node&)> parser, const std::string& errorMessage = "") {...}
   
   std::string type = parseRequiredField(&controllerServiceNode, {"class", "type"}, logger_, CONFIG_YAML_CONTROLLER_SERVICES_KEY, [] (const YAML::Node& node) {return node.as<std::string>();});
   ```




-- 
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