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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
