craigrosie commented on a change in pull request #7227: [AIRFLOW-6530] Allow
Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r391939968
##########
File path: airflow/stats.py
##########
@@ -200,10 +200,35 @@ def __init__(self, *args, **kwargs):
self.__class__.instance = DummyStatsLogger()
except (socket.gaierror, ImportError) as e:
log.warning("Could not configure StatsClient: %s, using
DummyStatsLogger instead.", e)
+ self.__class__.instance = DummyStatsLogger()
Review comment:
I think I found a bug when adding tests for the new functionality. The log
message above the line I added states that the class used for the statsd client
should fall back to `DummyStatsLogger` if the defined statsd client class
cannot be imported, or cannot connect to a statsd instance, but this never
happens, and `self.__class__.instance` remains set to the default value of
`None`
For example, if I try to load an invalid custom statsd class using the
functionality that @envious added, triggering an `ImportError`, when I then try
and call `Stats.incr()` (which should still work because the underlying class
has fallen back to `DummyStatsLogger`), I get errors like:
```python
cls = <class 'airflow.stats.Stats'>, name = 'incr'
def __getattr__(cls, name):
> return getattr(cls.instance, name)
E AttributeError: 'NoneType' object has no attribute 'incr'
airflow/stats.py:188: AttributeError
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services