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