[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #4221: [TE] Code cleanup - Pull out all validation checks from translator into validator modules

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

jihaozh commented on a change in pull request #4221: [TE] Code cleanup - Pull out all validation checks from translator into validator modules
URL: https://github.com/apache/incubator-pinot/pull/4221#discussion_r285710466
 
 

 ##########
 File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlDetectionConfigTranslator.java
 ##########
 @@ -77,7 +81,7 @@ public YamlDetectionConfigTranslator withExistingDetectionConfig(DetectionConfig
    * Fill in common fields of detection config. Properties of the pipeline is filled by the subclass.
    */
   public DetectionConfigDTO generateDetectionConfig() {
-    validateYAML(yamlConfig);
+    detectionValidator.validateConfig(yamlConfig);
 
 Review comment:
   How about calling detectionValidator in   CompositePipelineConfigTranslator.java's constructor. After this refactoring, each YamlDetectionConfigTranslator implementation has a corresponding DetectionConfigValidator to handle different YAML formats. Because YamlDetectionConfigTranslator is loaded dynamically using reflection, it can call the respective validator. 
   
   For example, if we plan to add EntityLevelPipelineConfigTranslator, it can call EntityLevelDetectionValidator to validate the YAML.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org