You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/01/21 10:41:25 UTC

[GitHub] [airflow] envious opened a new pull request #7227: [AIRFLOW-6539] Allow Custom Statsd Client

envious opened a new pull request #7227: [AIRFLOW-6539] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227
 
 
   This change allows Airflow users to utilise their own custom Statsd client as discussed [here](https://issues.apache.org/jira/browse/AIRFLOW-6530).
   
   Many companies have their own custom Statsd clients and this change should allow for easier adoption of Airflow by large corporations.
   
   Example usage based off of this change:
   
   1) User adds a module path to their airflow.cfg file next to the 'statsd_custom_client_path' key in the scheduler section of the config.
   
   As such:
   `statsd_custom_client_path = company.statsdclient.customclient`
   
   2) Ensure the newly added client exists on the PYTHONPATH.
   
   ---
   Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
   
   Make sure to mark the boxes below before creating PR: [x]
   
   - [x] Description above provides context of the change
   - [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
   - [ ] Unit tests coverage for changes (not needed for documentation changes)
   - [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [x] Relevant documentation is updated including usage instructions.
   - [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   <sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
   
   ---
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-577151472
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=h1) Report
   > Merging [#7227](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/2ea9278f76bf71aafb5601160602bf7f4194242f&el=desc) will **decrease** coverage by `0.66%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7227/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7227      +/-   ##
   ==========================================
   - Coverage   86.83%   86.16%   -0.67%     
   ==========================================
     Files         897      915      +18     
     Lines       42751    44164    +1413     
   ==========================================
   + Hits        37121    38055     +934     
   - Misses       5630     6109     +479     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/stats.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9zdGF0cy5weQ==) | `86.07% <100.00%> (+2.51%)` | :arrow_up: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
   | [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
   | [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55.00% <0.00%> (-45.00%)` | :arrow_down: |
   | ... and [86 more](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=footer). Last update [2ea9278...0bc17c3](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] craigrosie commented on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
craigrosie commented on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-598097967
 
 
   Hey @kaxil / @mik-laj, are you able to give this another review now that we've added tests? 😄 Thank you in advance!

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r392709260
 
 

 ##########
 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()
 
     def get_statsd_logger(self):
-        from statsd import StatsClient
-        statsd = StatsClient(
+        if conf.getboolean('scheduler', 'statsd_on'):
+
+            from statsd import StatsClient
+
+            if conf.has_option('scheduler', 'statsd_custom_client_path'):
+
+                custom_statsd_module_path = conf.get('scheduler', 'statsd_custom_client_path')
+
+                try:
+                    stats_class = import_string(custom_statsd_module_path)
+
 
 Review comment:
   ```suggestion
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] craigrosie commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil merged pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r392709232
 
 

 ##########
 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()
 
     def get_statsd_logger(self):
-        from statsd import StatsClient
-        statsd = StatsClient(
+        if conf.getboolean('scheduler', 'statsd_on'):
+
 
 Review comment:
   ```suggestion
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-581120176
 
 
   @envious - You can add another test at https://github.com/apache/airflow/blob/master/tests/test_stats.py by creating a new Statsd client and passing it using `conf_vars` like 
   
   https://github.com/apache/airflow/blob/cbcc22a2c2d5659bee0474d99af0dc24d4745a40/tests/providers/email/operators/test_email.py#L59-L64
   
   and asserting that the client is the same that you were expecting. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil edited a comment on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-581120176
 
 
   @envious - You can add another test at https://github.com/apache/airflow/blob/master/tests/test_stats.py by creating a new Statsd client and passing it using `conf_vars` like 
   
   https://github.com/apache/airflow/blob/d7d2794d05f44b6dd7edf2d44633cba9951e3db0/tests/utils/test_email.py#L90-L98
   
   and asserting that the client is the same that you were expecting. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on a change in pull request #7227: [AIRFLOW-6539] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7227: [AIRFLOW-6539] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r369150685
 
 

 ##########
 File path: airflow/config_templates/config.yml
 ##########
 @@ -1314,6 +1314,15 @@
       type: string
       example: ~
       default: ""
+    - name: statsd_custom_client_path
+      description: |
+        If you want to utilise your own custom Statsd client set the relevant
+        module path below. Note: The module path must exist on your PYTHONPATH
 
 Review comment:
   ```suggestion
           module path below. 
           Note: The module path must exist on your PYTHONPATH for Airflow to pick it up
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] envious commented on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
envious commented on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-577049260
 
 
   @kaxil thanks for your suggested changes, i've applied them. I'll figure out how to best test these changes today. Any clues?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] mik-laj commented on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-581168859
 
 
   > Many companies have their own custom Statsd clients and this change should allow for easier adoption of Airflow by large corporations.
   
   Can you say something more about that? Do you mean any public software or services?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] boring-cyborg[bot] commented on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-599260333
 
 
   Awesome work, congrats on your first merged pull request!
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r392498227
 
 

 ##########
 File path: airflow/config_templates/default_airflow.cfg
 ##########
 @@ -674,6 +674,12 @@ statsd_datadog_enabled = False
 # List of datadog tags attached to all metrics(e.g: key1:value1,key2:value2)
 statsd_datadog_tags =
 
+# If you want to utilise your own custom Statsd client set the relevant
+# module path below.
+# Note: The module path must exist on your PYTHONPATH for Airflow to pick it up
+# for Airflow to pick it up
 
 Review comment:
   ```suggestion
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io commented on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7227: [AIRFLOW-6539] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-577151472
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=h1) Report
   > Merging [#7227](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/5b44c0fc4ac0030c78ab6ad4e658e9abdf60dc04?src=pr&el=desc) will **decrease** coverage by `0.49%`.
   > The diff coverage is `9.09%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7227/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master    #7227     +/-   ##
   =========================================
   - Coverage   85.53%   85.04%   -0.5%     
   =========================================
     Files         756      791     +35     
     Lines       39859    40134    +275     
   =========================================
   + Hits        34094    34132     +38     
   - Misses       5765     6002    +237
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/stats.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9zdGF0cy5weQ==) | `64.44% <9.09%> (-6.81%)` | :arrow_down: |
   | [airflow/operators/postgres\_operator.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvcG9zdGdyZXNfb3BlcmF0b3IucHk=) | `0% <0%> (-100%)` | :arrow_down: |
   | [airflow/contrib/operators/snowflake\_operator.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL29wZXJhdG9ycy9zbm93Zmxha2Vfb3BlcmF0b3IucHk=) | `0% <0%> (-95.84%)` | :arrow_down: |
   | [airflow/contrib/hooks/snowflake\_hook.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3Nub3dmbGFrZV9ob29rLnB5) | `0% <0%> (-81.14%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `45.25% <0%> (-46.72%)` | :arrow_down: |
   | [airflow/kubernetes/refresh\_config.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3JlZnJlc2hfY29uZmlnLnB5) | `50.98% <0%> (-23.53%)` | :arrow_down: |
   | [airflow/api/client/local\_client.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY2xpZW50L2xvY2FsX2NsaWVudC5weQ==) | `92% <0%> (-8%)` | :arrow_down: |
   | [airflow/api/client/api\_client.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvY2xpZW50L2FwaV9jbGllbnQucHk=) | `63.15% <0%> (-1.55%)` | :arrow_down: |
   | ... and [78 more](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=footer). Last update [5b44c0f...fb21b77](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r392709252
 
 

 ##########
 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()
 
     def get_statsd_logger(self):
-        from statsd import StatsClient
-        statsd = StatsClient(
+        if conf.getboolean('scheduler', 'statsd_on'):
+
+            from statsd import StatsClient
+
+            if conf.has_option('scheduler', 'statsd_custom_client_path'):
+
 
 Review comment:
   ```suggestion
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] craigrosie commented on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
craigrosie commented on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-596606567
 
 
   While tests are being worked on for this, do you have any advice on how/if we can get this "backported" to the next 1.10 release?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-596679384
 
 
   > While tests are being worked on for this, do you have any advice on how/if we can get this "backported" to the next 1.10 release?
   
   marked it for 1.10.10. We will cherry-pick and backport your changes after merging it to master

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-577151472
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=h1) Report
   > Merging [#7227](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/2ea9278f76bf71aafb5601160602bf7f4194242f?src=pr&el=desc) will **decrease** coverage by `22.37%`.
   > The diff coverage is `71.16%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7227/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #7227       +/-   ##
   ===========================================
   - Coverage   86.83%   64.45%   -22.38%     
   ===========================================
     Files         897      903        +6     
     Lines       42751    43735      +984     
   ===========================================
   - Hits        37121    28189     -8932     
   - Misses       5630    15546     +9916
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ders/google/cloud/example\_dags/example\_dataflow.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2V4YW1wbGVfZGFncy9leGFtcGxlX2RhdGFmbG93LnB5) | `0% <ø> (-100%)` | :arrow_down: |
   | [airflow/serialization/serialized\_objects.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9zZXJpYWxpemF0aW9uL3NlcmlhbGl6ZWRfb2JqZWN0cy5weQ==) | `81.08% <ø> (-9.13%)` | :arrow_down: |
   | [...\_platform/example\_dags/example\_campaign\_manager.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL21hcmtldGluZ19wbGF0Zm9ybS9leGFtcGxlX2RhZ3MvZXhhbXBsZV9jYW1wYWlnbl9tYW5hZ2VyLnB5) | `93.93% <ø> (ø)` | :arrow_up: |
   | [...rflow/providers/amazon/aws/operators/sftp\_to\_s3.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYW1hem9uL2F3cy9vcGVyYXRvcnMvc2Z0cF90b19zMy5weQ==) | `100% <ø> (ø)` | :arrow_up: |
   | [airflow/providers/sftp/operators/sftp.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvc2Z0cC9vcGVyYXRvcnMvc2Z0cC5weQ==) | `87.67% <ø> (ø)` | :arrow_up: |
   | [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `75.35% <ø> (-19.72%)` | :arrow_down: |
   | [...e/marketing\_platform/operators/campaign\_manager.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL21hcmtldGluZ19wbGF0Zm9ybS9vcGVyYXRvcnMvY2FtcGFpZ25fbWFuYWdlci5weQ==) | `93.63% <ø> (ø)` | :arrow_up: |
   | [airflow/providers/google/cloud/hooks/gcs.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2hvb2tzL2djcy5weQ==) | `85.46% <ø> (-0.06%)` | :arrow_down: |
   | [...cncf/kubernetes/example\_dags/example\_kubernetes.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvY25jZi9rdWJlcm5ldGVzL2V4YW1wbGVfZGFncy9leGFtcGxlX2t1YmVybmV0ZXMucHk=) | `0% <0%> (-78.58%)` | :arrow_down: |
   | [...s/google/cloud/example\_dags/example\_datacatalog.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL2V4YW1wbGVfZGFncy9leGFtcGxlX2RhdGFjYXRhbG9nLnB5) | `0% <0%> (ø)` | |
   | ... and [538 more](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=footer). Last update [2ea9278...b909492](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#issuecomment-577151472
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=h1) Report
   > Merging [#7227](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/2ea9278f76bf71aafb5601160602bf7f4194242f?src=pr&el=desc) will **decrease** coverage by `0.66%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7227/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7227      +/-   ##
   ==========================================
   - Coverage   86.83%   86.16%   -0.67%     
   ==========================================
     Files         897      915      +18     
     Lines       42751    44164    +1413     
   ==========================================
   + Hits        37121    38055     +934     
   - Misses       5630     6109     +479
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/stats.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9zdGF0cy5weQ==) | `86.07% <100%> (+2.51%)` | :arrow_up: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0%> (-72.16%)` | :arrow_down: |
   | [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0%> (-64.16%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50% <0%> (-50%)` | :arrow_down: |
   | [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50% <0%> (-50%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
   | [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0%> (-46.67%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55% <0%> (-45%)` | :arrow_down: |
   | ... and [86 more](https://codecov.io/gh/apache/airflow/pull/7227/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=footer). Last update [2ea9278...0bc17c3](https://codecov.io/gh/apache/airflow/pull/7227?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on a change in pull request #7227: [AIRFLOW-6539] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7227: [AIRFLOW-6539] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r369150837
 
 

 ##########
 File path: airflow/config_templates/default_airflow.cfg
 ##########
 @@ -631,6 +631,11 @@ statsd_prefix = airflow
 # start with the elements of the list (e.g: scheduler,executor,dagrun)
 statsd_allow_list =
 
+# If you want to utilise your own custom Statsd client set the relevant module
+# path below
+# Note: The module path must exist on your PYTHONPATH for Airflow to pick it up
+statsd_custom_client_path =
 
 Review comment:
   ```suggestion
   # statsd_custom_client_path =
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] craigrosie commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
craigrosie commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r391940847
 
 

 ##########
 File path: airflow/config_templates/config.yml
 ##########
 @@ -1401,6 +1401,16 @@
       type: string
       example: ~
       default: ""
+    - name: statsd_custom_client_path
+      description: |
+        If you want to utilise your own custom Statsd client set the relevant
+        module path below.
+        Note: The module path must exist on your PYTHONPATH for Airflow to pick it up
+        for Airflow to pick it up
 
 Review comment:
   Thanks, fixed! 😄 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r391741050
 
 

 ##########
 File path: airflow/config_templates/config.yml
 ##########
 @@ -1401,6 +1401,16 @@
       type: string
       example: ~
       default: ""
+    - name: statsd_custom_client_path
+      description: |
+        If you want to utilise your own custom Statsd client set the relevant
+        module path below.
+        Note: The module path must exist on your PYTHONPATH for Airflow to pick it up
+        for Airflow to pick it up
 
 Review comment:
   Duplicate wording

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7227: [AIRFLOW-6530] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r391741603
 
 

 ##########
 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:
   Why this change?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on a change in pull request #7227: [AIRFLOW-6539] Allow Custom Statsd Client

Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #7227: [AIRFLOW-6539] Allow Custom Statsd Client
URL: https://github.com/apache/airflow/pull/7227#discussion_r369149968
 
 

 ##########
 File path: airflow/config_templates/config.yml
 ##########
 @@ -1314,6 +1314,15 @@
       type: string
       example: ~
       default: ""
+    - name: statsd_custom_client_path
+      description: |
+        If you want to utilise your own custom Statsd client set the relevant
+        module path below. Note: The module path must exist on your PYTHONPATH
+        for Airflow to pick it up
+      version_added: ~
+      type: string
+      example: ~
+      default: ""
 
 Review comment:
   ```suggestion
         default: ~
   ```
   
   As it is optional option so let's keep it as **null**

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services