Re: [PR] Support a Unix Domain Socket for the metrics clients [airflow]

["xBis7 (via GitHub)" <gi...@apache.org>]

xBis7 commented on code in PR #70517:
URL: https://github.com/apache/airflow/pull/70517#discussion_r3721048824


##########
airflow-core/src/airflow/config_templates/config.yml:
##########
@@ -1195,11 +1195,14 @@ metrics:
       default: "False"
     statsd_host:
       description: |
-        Specifies the host address where the StatsD daemon (or server) is running
+        Specifies the host address where the StatsD daemon (or server) is running. Left
+        unset by default so the DataDog client can fall back to its own environment
+        variables; the StatsD client defaults to ``localhost``. If ``statsd_socket_path`` is also
+        set, the socket takes precedence and a warning is logged.
       version_added: 2.0.0
       type: string
       example: ~
-      default: "localhost"
+      default: ~

Review Comment:
   Now all three, host, port and socket path, default to empty. Is this the desired behavior? It seems to me that it will break existing deployments.
   
   I think we should still keep the default for host and port to not affect any users that rely on them but don't specify a particular value and make the new addition fallback to empty if not provided. If provided, then we can check host and port and log a warning if they have a value.



-- 
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-unsubscribe@airflow.apache.org

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