[AIRFLOW-1526] Add dingding hook and operator #4895

zhongjiajie · 2019-03-11T02:22:47Z

Dingding is popular team collaboration tools talk
just like Slack. This PR is add it to master, could
help us easy send message to Dingding.

Change db.py add conn dingding_webhook_default, could
easy configure dingding message due the host is constant

Add how to operator dingding section tell users
how to send different kind of dingding message or
how to use dingding as callback funtion in DAG
and add example to show how to use.

Set operator UI color as dingding icon color

Make sure you have checked all steps below.

Jira

My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
- In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.
- https://issues.apache.org/jira/browse/AIRFLOW-1526
- In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal(AIP).

Description

Here are some details about my PR, including screenshots of any UI changes:

Add dingding (popular team collaboration tools in China) hook and operator to master,

Tests

My PR adds the following unit tests :

Commits

My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters (not including Jira issue reference)
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"

Documentation

In case of new functionality, my PR adds documentation that describes how to use it.
- When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

Code Quality

Passes flake8

mik-laj · 2019-03-11T02:38:51Z

Do you think it is a good idea to allow tokens to be passed as an operator parameter? For me it is not safe.
First, operator parameters are not stored securely. Connection use a fernet
Reference: https://airflow.readthedocs.io/en/latest/howto/secure-connections.html#rotating-encryption-keys
Second: operators parameters is available in UI. This extends the group of people who have access to the token.

Similar comment from today: https://github.com/apache/airflow/pull/4891/files#r264048383

zhongjiajie · 2019-03-11T02:49:17Z

@mik-laj This PR have to way to pass token, One is pass token by Operator, another is set it in dingding_webhook_default connection.
Second way have no safe problem, allow pass to operator just provide other way to send message.

tests/contrib/operators/test_dingding_webhook_operator.py

mik-laj · 2019-03-11T03:26:04Z

I know there are two ways but I think one of them is not safe and should not be supported. First, security should be a standard, not an option to choose from. people are too lazy and therefore do not make good choices about security. Second, the code of operators that is in the core is used to learning and therefore it should not have security flaws.

I develop a operators for GCP Transfer Service. and I intentionally blocked the passing of passwords as parameters to limit the misuse of the operator.
https://github.com/apache/airflow/pull/4792/files#diff-1f5fe79c9bd42285838bf61cf696ea2bR136

zhongjiajie · 2019-03-11T03:40:00Z

@mik-laj Ok, your right, I will remove pass token from operator. do you think I should add some docs to describe in howto let people know how to use this hook or operator?

XD-DENG · 2019-03-11T03:47:56Z

The implementation is a bit tricky to me. It's dependent on the Dingtalk's single API endpoint (https://oapi.dingtalk.com).

The scope of this PR itself is similar to Slack-related hooks/operators. However, they were implemented on top of slackclient (https://github.com/apache/airflow/blob/master/airflow/hooks/slack_hook.py)

mik-laj · 2019-03-11T04:00:12Z

@XD-DENG we have a two hooks for Slack :-) Look at https://github.com/apache/airflow/blob/master/airflow/contrib/hooks/slack_webhook_hook.py.

mik-laj · 2019-03-11T04:05:19Z

@zhongjiajie The howto documentation is recommended, but not required. If you have the time and desire then I will be happy when you write.
The guide should contain such sections:

short description of service,
the scenario - not required
prerequisite tasks,
how to use.

Good example is: https://boto3.amazonaws.com/v1/documentation/api/latest/guide/s3-example-creating-buckets.html

Fragment of the code should be attached using the literalinclude directive. It allows its automatic validation and allows to add link to full file ( #4894 )

Currently, the guides do not contain all sections, but I plan to develop them.

airflow/contrib/hooks/dingding_webhook_hook.py

zhongjiajie · 2019-03-11T04:31:41Z

The implementation is a bit tricky to me. It's dependent on the Dingtalk's single API endpoint (https://oapi.dingtalk.com).

The scope of this PR itself is similar to Slack-related hooks/operators. However, they were implemented on top of slackclient (https://github.com/apache/airflow/blob/master/airflow/hooks/slack_hook.py)

@XD-DENG slack have officail client, but dingtalk not, as @mik-laj said we have two slack hook, one is base on slackclient another are on webhook.

zhongjiajie · 2019-03-13T05:52:01Z

CI failed unrelated

zhongjiajie · 2019-03-16T16:09:17Z

After today work, I think this PR is ready to review. PTAL @XD-DENG @Fokko thanks.

codecov-io · 2019-03-17T04:34:57Z

Codecov Report

Merging #4895 into master will decrease coverage by 0.12%.
The diff coverage is 57.5%.

@@            Coverage Diff             @@
##           master    #4895      +/-   ##
==========================================
- Coverage   75.81%   75.69%   -0.13%     
==========================================
  Files         458      461       +3     
  Lines       29882    29935      +53     
==========================================
+ Hits        22655    22658       +3     
- Misses       7227     7277      +50

Impacted Files	Coverage Δ
.../contrib/example_dags/example_dingding_operator.py	`0% <0%> (ø)`
airflow/contrib/operators/dingding_operator.py	`100% <100%> (ø)`
airflow/utils/db.py	`90.29% <100%> (+0.09%)`	⬆️
airflow/contrib/hooks/dingding_hook.py	`66.66% <66.66%> (ø)`
airflow/contrib/kubernetes/secret.py	`25% <0%> (-44.24%)`	⬇️
airflow/contrib/hooks/aws_athena_hook.py	`60% <0%> (-5.46%)`	⬇️
airflow/contrib/operators/aws_athena_operator.py	`70.45% <0%> (-3.02%)`	⬇️
airflow/stats.py	`68.18% <0%> (-1.52%)`	⬇️
airflow/jobs.py	`77.49% <0%> (-1.35%)`	⬇️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4e24e3...372c6ce. Read the comment docs.

docs/howto/operator/dingding.rst

zhongjiajie · 2019-03-22T05:31:57Z

@mik-laj Add prerequisites,
@XD-DENG PTAL, Thanks.

airflow/contrib/operators/dingding_webhook_operator.py

airflow/utils/db.py

airflow/contrib/hooks/dingding_webhook_hook.py

ashb · 2019-03-24T12:07:26Z

I edited the docs a bit directly on your branch to improve the English a bit (some of the tenses and plurals were a bit wrong) and to update the instructions about setting up the hook: 19133bc

(We don't need to set the host anymore do we?)

zhongjiajie · 2019-03-24T13:51:33Z

@ashb Thank for you improvement. About the host, we still have code in
https://github.com/apache/airflow/pull/4895/files#diff-0ddca7f4132e6c4a5dc1ee70d4008085R108
so I think we should let user know their could manual pass host to connection. WDYT?

zhongjiajie · 2019-03-24T13:54:46Z

BTW, I am trying improve my English recently 😭

ashb · 2019-03-24T14:48:02Z

so I think we should let user know their could manual pass host to connection. WDYT?

Sounds good.

zhongjiajie · 2019-03-24T14:53:21Z

so I think we should let user know their could manual pass host to connection. WDYT?

Sounds good.

So should I revert describe in https://github.com/apache/airflow/pull/4895/files#diff-a46bd26d7b5231c209f6fa37d2622cc5R29 to tell user could pass host to connection?

ashb · 2019-03-24T14:55:28Z

Not revert, but say something like "if you need to change the host used you can set the host field of the connection". (Most people wont need to do this, so the docs shouldn't imply its a required step)

zhongjiajie · 2019-03-24T15:09:50Z

I update this PR, PTAL, Thanks.

zhongjiajie · 2019-03-25T15:04:54Z

@ashb PTAL,
CI failed unrelated, it passed in my fork https://travis-ci.com/zhongjiajie/airflow/builds/105652590?utm_medium=notification&utm_source=email

ashb · 2019-03-29T11:02:11Z

@zhongjiajie Could you rebase please?

Dingding is popular team collaboration tools talk just like Slack. This PR is add it to master, could help us easy send message to Dingding. Change db.py add conn dingding_webhook_default, could easy configure dingding message due the host is constant Add how to operator dingding section tell users how to send different kind of dingding message or how to use dingding as callback funtion in DAG and add example to show how to use. Set operator UI color as dingding icon color

zhongjiajie

@ashb I not really sure about the auto api change by @mik-laj , it's that mean we no need to add any content for auto generate api doc?

zhongjiajie · 2019-03-29T11:25:28Z

docs/howto/operator/index.rst

@@ -28,6 +28,7 @@ information.
 .. toctree::
    :maxdepth: 2

-    python


Alphabetical order operator

ashb · 2019-03-29T11:32:19Z

it's that mean we no need to add any content for auto generate api doc?

Correct, it should automatically picked up from the doc comments on the module and linked.

zhongjiajie · 2019-03-29T11:33:06Z

@ashb Thanks.

zhongjiajie · 2019-03-29T13:30:20Z

@ashb FYI, CI green

zhongjiajie · 2019-03-29T13:31:08Z

BTW, could we cherry-pick this PR to 1.10.3 ?

Dingding is popular team collaboration tools talk just like Slack. This PR is add it to master, could help us easy send message to Dingding. Add how to operator dingding section tell users how to send different kind of dingding message or how to use dingding as callback funtion in DAG and add example to show how to use. Set operator UI color as dingding icon color

zhongjiajie · 2019-03-29T17:12:38Z

@ashb Thanks 👍

Dingding is popular team collaboration tools talk just like Slack. This PR is add it to master, could help us easy send message to Dingding. Add how to operator dingding section tell users how to send different kind of dingding message or how to use dingding as callback funtion in DAG and add example to show how to use. Set operator UI color as dingding icon color

mik-laj mentioned this pull request Mar 11, 2019

[AIRFLOW-4067] Telegram hook/operator to post messages to telegram channels #4891

Closed

6 tasks

mik-laj reviewed Mar 11, 2019

View reviewed changes

tests/contrib/operators/test_dingding_webhook_operator.py Outdated Show resolved Hide resolved

mik-laj reviewed Mar 11, 2019

View reviewed changes

airflow/contrib/hooks/dingding_webhook_hook.py Outdated Show resolved Hide resolved

zhongjiajie mentioned this pull request Mar 12, 2019

[AIRFLOW-4069] Add Opsgenie Alert Hook and Operator #4903

Merged

6 tasks

zhongjiajie force-pushed the hooks_add_dingding_hooks_op branch from a961a9d to 7956d0f Compare March 13, 2019 03:43

zhongjiajie force-pushed the hooks_add_dingding_hooks_op branch 2 times, most recently from e7cfb72 to aeb055a Compare March 13, 2019 12:04

zhongjiajie changed the title ~~[AIRFLOW-1526] Add dingding hook and operator~~ [WIP][AIRFLOW-1526] Add dingding hook and operator Mar 14, 2019

zhongjiajie force-pushed the hooks_add_dingding_hooks_op branch from aeb055a to 965d8c8 Compare March 16, 2019 16:07

zhongjiajie changed the title ~~[WIP][AIRFLOW-1526] Add dingding hook and operator~~ [AIRFLOW-1526] Add dingding hook and operator Mar 16, 2019

zhongjiajie force-pushed the hooks_add_dingding_hooks_op branch 4 times, most recently from 1c54cbb to fd7c2e5 Compare March 17, 2019 03:52

mik-laj reviewed Mar 20, 2019

View reviewed changes

docs/howto/operator/dingding.rst Outdated Show resolved Hide resolved

ashb requested changes Mar 22, 2019

View reviewed changes

zhongjiajie force-pushed the hooks_add_dingding_hooks_op branch 2 times, most recently from 5d2d7d9 to ccb4f21 Compare March 22, 2019 19:12

zhongjiajie force-pushed the hooks_add_dingding_hooks_op branch 2 times, most recently from f877fcc to 6c914de Compare March 25, 2019 11:08

zhongjiajie force-pushed the hooks_add_dingding_hooks_op branch from 6c914de to 372c6ce Compare March 29, 2019 11:22

ashb approved these changes Mar 29, 2019

View reviewed changes

zhongjiajie commented Mar 29, 2019

View reviewed changes

ashb merged commit 6148509 into apache:master Mar 29, 2019

zhongjiajie deleted the hooks_add_dingding_hooks_op branch March 29, 2019 17:08

Taragolis mentioned this pull request Sep 29, 2023

Change Dingding (DingTalk) examples and docs #34680

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AIRFLOW-1526] Add dingding hook and operator #4895

[AIRFLOW-1526] Add dingding hook and operator #4895

zhongjiajie commented Mar 11, 2019 •

edited

Loading

mik-laj commented Mar 11, 2019 •

edited

Loading

zhongjiajie commented Mar 11, 2019

mik-laj commented Mar 11, 2019 •

edited

Loading

zhongjiajie commented Mar 11, 2019

XD-DENG commented Mar 11, 2019

mik-laj commented Mar 11, 2019

mik-laj commented Mar 11, 2019 •

edited

Loading

zhongjiajie commented Mar 11, 2019 •

edited

Loading

zhongjiajie commented Mar 13, 2019

zhongjiajie commented Mar 16, 2019

codecov-io commented Mar 17, 2019 •

edited

Loading

zhongjiajie commented Mar 22, 2019

ashb commented Mar 24, 2019

zhongjiajie commented Mar 24, 2019

zhongjiajie commented Mar 24, 2019 •

edited

Loading

ashb commented Mar 24, 2019

zhongjiajie commented Mar 24, 2019

ashb commented Mar 24, 2019

zhongjiajie commented Mar 24, 2019

zhongjiajie commented Mar 25, 2019

ashb commented Mar 29, 2019

zhongjiajie left a comment

zhongjiajie Mar 29, 2019

ashb commented Mar 29, 2019 •

edited

Loading

zhongjiajie commented Mar 29, 2019

zhongjiajie commented Mar 29, 2019

zhongjiajie commented Mar 29, 2019 •

edited

Loading

zhongjiajie commented Mar 29, 2019

[AIRFLOW-1526] Add dingding hook and operator #4895

[AIRFLOW-1526] Add dingding hook and operator #4895

Conversation

zhongjiajie commented Mar 11, 2019 • edited Loading

Jira

Description

Tests

Commits

Documentation

Code Quality

mik-laj commented Mar 11, 2019 • edited Loading

zhongjiajie commented Mar 11, 2019

mik-laj commented Mar 11, 2019 • edited Loading

zhongjiajie commented Mar 11, 2019

XD-DENG commented Mar 11, 2019

mik-laj commented Mar 11, 2019

mik-laj commented Mar 11, 2019 • edited Loading

zhongjiajie commented Mar 11, 2019 • edited Loading

zhongjiajie commented Mar 13, 2019

zhongjiajie commented Mar 16, 2019

codecov-io commented Mar 17, 2019 • edited Loading

Codecov Report

zhongjiajie commented Mar 22, 2019

ashb commented Mar 24, 2019

zhongjiajie commented Mar 24, 2019

zhongjiajie commented Mar 24, 2019 • edited Loading

ashb commented Mar 24, 2019

zhongjiajie commented Mar 24, 2019

ashb commented Mar 24, 2019

zhongjiajie commented Mar 24, 2019

zhongjiajie commented Mar 25, 2019

ashb commented Mar 29, 2019

zhongjiajie left a comment

Choose a reason for hiding this comment

zhongjiajie Mar 29, 2019

Choose a reason for hiding this comment

ashb commented Mar 29, 2019 • edited Loading

zhongjiajie commented Mar 29, 2019

zhongjiajie commented Mar 29, 2019

zhongjiajie commented Mar 29, 2019 • edited Loading

zhongjiajie commented Mar 29, 2019

zhongjiajie commented Mar 11, 2019 •

edited

Loading

mik-laj commented Mar 11, 2019 •

edited

Loading

mik-laj commented Mar 11, 2019 •

edited

Loading

mik-laj commented Mar 11, 2019 •

edited

Loading

zhongjiajie commented Mar 11, 2019 •

edited

Loading

codecov-io commented Mar 17, 2019 •

edited

Loading

zhongjiajie commented Mar 24, 2019 •

edited

Loading

ashb commented Mar 29, 2019 •

edited

Loading

zhongjiajie commented Mar 29, 2019 •

edited

Loading