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


##########
devel-common/src/tests_common/test_utils/otel_utils.py:
##########
@@ -24,12 +24,12 @@
 
 from sqlalchemy import inspect, select
 
-from airflow.models import Base
-
 log = logging.getLogger("integration.otel.otel_utils")
 
 
 def dump_airflow_metadata_db(session):
+    from airflow.models import Base

Review Comment:
   Isn't this still an issue even if the import is now local?



##########
shared/observability/tests/observability/metrics/test_stats.py:
##########
@@ -497,64 +491,64 @@ def always_valid(stat_name):
 
 
 class TestCustomStatsName:
-    @conf_vars(
-        {
-            ("metrics", "statsd_on"): "True",
-            ("metrics", "stat_name_handler"): 
"unit.observability.metrics.test_stats.always_invalid",
-        }
-    )
-    @mock.patch("statsd.StatsClient")
-    def test_does_not_send_stats_using_statsd_when_the_name_is_not_valid(self, 
mock_statsd):
-        importlib.reload(airflow_shared.observability.metrics.stats)
-        airflow_shared.observability.metrics.stats.Stats.incr("empty_key")
-        mock_statsd.return_value.assert_not_called()
+    def test_does_not_send_stats_using_statsd_when_the_name_is_not_valid(self):
+        with mock.patch("statsd.StatsClient") as mock_statsd:
+            importlib.reload(airflow_shared.observability.metrics.stats)
+            airflow_shared.observability.metrics.stats.Stats.initialize(
+                factory=get_statsd_logger_factory(
+                    stats_class=mock_statsd,
+                    stat_name_handler=always_invalid,
+                )
+            )
+            airflow_shared.observability.metrics.stats.Stats.incr("empty_key")
+            mock_statsd.return_value.assert_not_called()
 
     @skip_if_force_lowest_dependencies_marker
-    @conf_vars(
-        {
-            ("metrics", "statsd_datadog_enabled"): "True",
-            (
-                "metrics",
-                "stat_name_handler",
-            ): "observability.metrics.test_stats.always_invalid",
-        }
-    )
-    @mock.patch("datadog.DogStatsd")
-    def 
test_does_not_send_stats_using_dogstatsd_when_the_name_is_not_valid(self, 
mock_dogstatsd):
-        importlib.reload(airflow_shared.observability.metrics.stats)
-        airflow_shared.observability.metrics.stats.Stats.incr("empty_key")
-        mock_dogstatsd.return_value.assert_not_called()
+    def 
test_does_not_send_stats_using_dogstatsd_when_the_name_is_not_valid(self):
+        with mock.patch("datadog.DogStatsd") as mock_dogstatsd:
+            importlib.reload(airflow_shared.observability.metrics.stats)
+            airflow_shared.observability.metrics.stats.Stats.initialize(
+                factory=lambda: datadog_logger.get_dogstatsd_logger(

Review Comment:
   Now that `get_dogstatsd_logger` is used more that once, it could be 
refactored into a method like `get_statsd_logger_factory`. WDYT?



-- 
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]

Reply via email to