ferruzzi commented on code in PR #41975:
URL: https://github.com/apache/airflow/pull/41975#discussion_r1744336098


##########
airflow/metrics/validators.py:
##########
@@ -118,7 +107,7 @@ def get_validator() -> ListValidator:
     else:
         list_type = DEFAULT_VALIDATOR_TYPE
 
-    return validators[validator_type][list_type](metric_lists[list_type])
+    return validators[list_type](metric_lists[list_type])

Review Comment:
   Or skip the DEFAULT_VALIDATOR_TYPE entirely and do something like
   
   ```
   def get_validator():
       metric_allow_list = conf.get("metrics", "metrics_allow_list", 
fallback=None)
       metric_block_list = conf.get("metrics", "metrics_block_list", 
fallback=None)
   
       if metric_allow_list and metric_block_list:
           # If both are set, warn
           log.warning("Ignoring metrics_block_list as both metrics_allow_list 
and metrics_block_list have been set.")
       elif metric_block_list:
          # If not both but block is set, use that
          return PatternBlockListValidator(metric_block_list)
   
       # if none set, both are set, or only only allow is set then use allow
       return PatternAllowListValidator(metric_allow_list)        
   ```        
   
   
   But maybe that's more confusing and the more verbose way is easier to 
understand.    I don't know, I'll leave it up to you unless someone else chimes 
in with stronger opinions.



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to