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 2021/09/03 08:01:07 UTC

[GitHub] [airflow] ephraimbuddy commented on a change in pull request #17980: Implement API endpoint for DAG deletion

ephraimbuddy commented on a change in pull request #17980:
URL: https://github.com/apache/airflow/pull/17980#discussion_r701676584



##########
File path: airflow/api_connexion/endpoints/dag_endpoint.py
##########
@@ -100,3 +102,20 @@ def patch_dag(session, dag_id, update_mask=None):
     setattr(dag, 'is_paused', patch_body['is_paused'])
     session.commit()
     return dag_schema.dump(dag)
+
+
+@security.requires_access([(permissions.ACTION_CAN_DELETE, permissions.RESOURCE_DAG)])
+@provide_session
+def delete_dag(dag_id: str, session: Session):
+    """Delete the specific DAG."""
+    # TODO: This function is shared with the /delete endpoint used by the web
+    # UI, so we're reusing it to simplify maintenance. Refactor the function to
+    # another place when the experimental/legacy API is removed.
+    from airflow.api.common.experimental import delete_dag
+
+    try:
+        delete_dag.delete_dag(dag_id, session=session)
+    except DagNotFound:
+        raise NotFound(f"Dag with id: '{dag_id}' not found")
+

Review comment:
       The delete_dag function also raises AirflowException when the dag still has running task instances. I think we should also handle the exception here and tell the user to mark those task instances as failed or successful before deleting, just like we do in the webserver. What do you think?

##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -478,6 +478,23 @@ paths:
         '404':
           $ref: '#/components/responses/NotFound'
 
+    delete:
+      summary: Delete a DAG

Review comment:
       ```suggestion
         summary: Delete a DAG
         description: This endpoint will delete ALL metadata, DAGRuns, etc EXCEPT Log. This action cannot be undone
   ```
   I think that we should add this description as we do in the UI, so users are aware of what this can cause 




-- 
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