xBis7 commented on code in PR #70517:
URL: https://github.com/apache/airflow/pull/70517#discussion_r3812573626
##########
shared/observability/src/airflow_shared/observability/metrics/statsd_logger.py:
##########
@@ -170,21 +191,47 @@ def get_statsd_logger(
stat_name_handler: Callable[[str], str] | None = None,
statsd_influxdb_enabled: bool = False,
) -> SafeStatsdLogger:
- """Return logger for StatsD."""
- statsd = stats_class(
- host=host,
- port=port,
- prefix=prefix,
- ipv6=ipv6,
+ """Return a UDP StatsD logger."""
+ # Local import to avoid requiring statsd when other backends are used
(e.g. Datadog)
+ from statsd import StatsClient as _StatsClient
Review Comment:
Isn't the import under the type checking block enough?
##########
shared/observability/src/airflow_shared/observability/metrics/statsd_logger.py:
##########
@@ -170,21 +191,47 @@ def get_statsd_logger(
stat_name_handler: Callable[[str], str] | None = None,
statsd_influxdb_enabled: bool = False,
) -> SafeStatsdLogger:
- """Return logger for StatsD."""
- statsd = stats_class(
- host=host,
- port=port,
- prefix=prefix,
- ipv6=ipv6,
+ """Return a UDP StatsD logger."""
+ # Local import to avoid requiring statsd when other backends are used
(e.g. Datadog)
Review Comment:
If we end up in this file, then the user has already enabled statsd in the
config. I would expect statsd to be available, but even if for some reason it
isn't and it's configured to be used, then it should fail. Isn't it a valid
scenario for a failure?
--
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]
