You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Taragolis (via GitHub)" <gi...@apache.org> on 2023/09/29 11:22:46 UTC

[GitHub] [airflow] Taragolis commented on a diff in pull request #34680: Remove bad advice from Dingding examples and docs

Taragolis commented on code in PR #34680:
URL: https://github.com/apache/airflow/pull/34680#discussion_r1341246191


##########
tests/system/providers/dingding/example_dingding.py:
##########
@@ -30,32 +30,9 @@
 DAG_ID = "example_dingding_operator"
 
 
-# [START howto_operator_dingding_failure_callback]
-def failure_callback(context):
-    """
-    The function that will be executed on failure.
-
-    :param context: The context of the executed task.
-    """
-    message = (
-        f"AIRFLOW TASK FAILURE TIPS:\n"
-        f"DAG:    {context['task_instance'].dag_id}\n"
-        f"TASKS:  {context['task_instance'].task_id}\n"
-        f"Reason: {context['exception']}\n"
-    )
-    return DingdingOperator(
-        task_id="dingding_success_callback",

Review Comment:
   I would be a good idea if we had any clue is this provider even work. That's why I just remove potentially harmful code rather than try to fix it.
   
   Just some information:
   1. It was added as part of contrib hooks/operators in https://github.com/apache/airflow/pull/4895, and after that I can't find any significant changes, only generic one: move to separate package,  some lint fixes and etc.
   2. Seems like this service this service should named as [DingTalk](https://en.wikipedia.org/wiki/DingTalk) rather than [Dingding](https://www.google.com/search?q=Dingding)
   3. Link in provider go to the api, which return 404 Error: https://oapi.dingtalk.com
   4. Documentation only available on Chinese: https://open-doc.dingtalk.com/microapp/serverapi2/qf2nxq
   5. Message for callback try to send exception, but DAG context doesn't have `exception` (at least in Airflow 2) 
   
   So maybe just remove this part and consider the possibility of suspend this provider after changes appear in the documentation. WDYT?
   
   I could change it by something that might work, but have no idea is other parts works or not.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org