Re: [PR] Add task failed dependencies to details page. [airflow]
jscheffl merged PR #38449: URL: https://github.com/apache/airflow/pull/38449 -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
tirkarthi commented on PR #38449: URL: https://github.com/apache/airflow/pull/38449#issuecomment-2115146585 @bbovenzi Thanks, added auto refresh as suggested and rebased with latest main branch. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
bbovenzi commented on PR #38449: URL: https://github.com/apache/airflow/pull/38449#issuecomment-2103044992 > @jscheffl Thanks, updated the version number and regenerated the files. I have looked into auto refresh and found a pattern where auto refresh is done based on `useAutoRefresh`. The API useTaskFailedDependency doesn't have the taskinstance state based on which the refresh should happen. Once the task goes to queued state the `showTaskSchedulingDependencies` flag should return False which should hide the component in UI. @bbovenzi Any pointers here? I think `useTaskFailedDependency` can just use the normal refresh logic: `refetchInterval: isRefreshOn && (autoRefreshInterval || 1) * 1000,`. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
tirkarthi commented on PR #38449: URL: https://github.com/apache/airflow/pull/38449#issuecomment-2102692338 @jscheffl Thanks, updated the version number and regenerated the files. I have looked into auto refresh and found a pattern where auto refresh is done based on `useAutoRefresh`. The API useTaskFailedDependency doesn't have the taskinstance state based on which the refresh should happen. Once the task goes to queued state the `showTaskSchedulingDependencies` flag should return False which should hide the component in UI. @bbovenzi Any pointers here? -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
bbovenzi commented on code in PR #38449:
URL: https://github.com/apache/airflow/pull/38449#discussion_r1592484656
##
airflow/www/static/js/types/api-generated.ts:
##
@@ -3002,6 +3047,66 @@ export interface operations {
};
};
};
+ /**
+ * Get task dependencies blocking task from getting scheduled.
+ *
+ * *New in version 2.9.0*
Review Comment:
yes this is generated from the api spec
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
jscheffl commented on code in PR #38449:
URL: https://github.com/apache/airflow/pull/38449#discussion_r1590149970
##
airflow/api_connexion/openapi/v1.yaml:
##
@@ -679,6 +679,71 @@ paths:
"404":
$ref: "#/components/responses/NotFound"
+ /dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/dependencies:
+parameters:
+ - $ref: "#/components/parameters/DAGID"
+ - $ref: "#/components/parameters/DAGRunID"
+ - $ref: "#/components/parameters/TaskID"
+
+get:
+ summary: Get task dependencies blocking task from getting scheduled.
+ description: |
+Get task dependencies blocking task from getting scheduled.
+
+*New in version 2.9.0*
+ x-openapi-router-controller:
airflow.api_connexion.endpoints.task_instance_endpoint
+ operationId: get_task_instance_dependencies
+ tags: [TaskInstance]
+
+ responses:
+"200":
+ description: Success.
+ content:
+application/json:
+ schema:
+$ref: "#/components/schemas/TaskInstanceDependencyCollection"
+"400":
+ $ref: "#/components/responses/BadRequest"
+"401":
+ $ref: "#/components/responses/Unauthenticated"
+"403":
+ $ref: "#/components/responses/PermissionDenied"
+"404":
+ $ref: "#/components/responses/NotFound"
+
+ ?
/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/{map_index}/dependencies
+ : parameters:
+ - $ref: "#/components/parameters/DAGID"
+ - $ref: "#/components/parameters/DAGRunID"
+ - $ref: "#/components/parameters/TaskID"
+ - $ref: "#/components/parameters/MapIndex"
+
+get:
+ summary: Get task dependencies blocking task from getting scheduled.
+ description: |
+Get task dependencies blocking task from getting scheduled.
+
+*New in version 2.9.0*
Review Comment:
+1 here
```suggestion
*New in version 2.10.0*
```
##
airflow/www/static/js/types/api-generated.ts:
##
@@ -3002,6 +3047,66 @@ export interface operations {
};
};
};
+ /**
+ * Get task dependencies blocking task from getting scheduled.
+ *
+ * *New in version 2.9.0*
Review Comment:
+1 here ... but this is generated code, correct? :-D
```suggestion
* *New in version 2.10.0*
```
##
airflow/www/static/js/types/api-generated.ts:
##
@@ -3002,6 +3047,66 @@ export interface operations {
};
};
};
+ /**
+ * Get task dependencies blocking task from getting scheduled.
+ *
+ * *New in version 2.9.0*
+ */
+ get_task_instance_dependencies: {
+parameters: {
+ path: {
+/** The DAG ID. */
+dag_id: components["parameters"]["DAGID"];
+/** The DAG run ID. */
+dag_run_id: components["parameters"]["DAGRunID"];
+/** The task ID. */
+task_id: components["parameters"]["TaskID"];
+ };
+};
+responses: {
+ /** Success. */
+ 200: {
+content: {
+ "application/json":
components["schemas"]["TaskInstanceDependencyCollection"];
+};
+ };
+ 400: components["responses"]["BadRequest"];
+ 401: components["responses"]["Unauthenticated"];
+ 403: components["responses"]["PermissionDenied"];
+ 404: components["responses"]["NotFound"];
+};
+ };
+ /**
+ * Get task dependencies blocking task from getting scheduled.
+ *
+ * *New in version 2.9.0*
Review Comment:
```suggestion
* *New in version 2.10.0*
```
##
airflow/api_connexion/openapi/v1.yaml:
##
@@ -679,6 +679,71 @@ paths:
"404":
$ref: "#/components/responses/NotFound"
+ /dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/dependencies:
+parameters:
+ - $ref: "#/components/parameters/DAGID"
+ - $ref: "#/components/parameters/DAGRunID"
+ - $ref: "#/components/parameters/TaskID"
+
+get:
+ summary: Get task dependencies blocking task from getting scheduled.
+ description: |
+Get task dependencies blocking task from getting scheduled.
+
+*New in version 2.9.0*
Review Comment:
Will need to go to 2.10 I assume
```suggestion
*New in version 2.10.0*
```
##
airflow/www/static/js/types/api-generated.ts:
##
@@ -139,6 +139,44 @@ export interface paths {
};
};
};
+ "/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/dependencies": {
+/**
+ * Get task dependencies blocking task from getting scheduled.
+ *
+ * *New in version 2.9.0*
+ */
+get: operations["get_task_instance_dependencies"];
+parameters: {
+ path: {
+/** The DAG ID. */
+dag_id: components["parameters"]["DAGID"];
+/** The DAG run ID. */
+dag_run_id: components["parameters"]["DAGRunID"];
+/** The task ID. */
+task_id: components["parameter
Re: [PR] Add task failed dependencies to details page. [airflow]
tirkarthi commented on PR #38449: URL: https://github.com/apache/airflow/pull/38449#issuecomment-2092365138 @bbovenzi Sorry, I got held up at work. Fixed the code comments to use a spinner like other pages and an alert on API error. Did a basic test with a mapped task which also seems to be working as expected. Screenshots as below. Please note that I have enabled this check for tasks in None state as well as there are cases like execution_date is greater than end_date that will be useful. In the old page I see this table always being shown so please let me know if this is needed always or task in scheduled state or task in None/scheduled state. Spinner during API call  API failed :  Table rendering :  -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
bbovenzi commented on PR #38449: URL: https://github.com/apache/airflow/pull/38449#issuecomment-2090939611 @tirkarthi Could you still rebase this? It would be great to get this merged soon so we can migrate all the /task pages. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
jscheffl commented on PR #38449: URL: https://github.com/apache/airflow/pull/38449#issuecomment-2067289934 I really like this extension in the view! Looking forward for fixes and removal of merge conflict... then would like to review. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
bbovenzi commented on code in PR #38449:
URL: https://github.com/apache/airflow/pull/38449#discussion_r1550063798
##
airflow/www/static/js/dag/details/taskInstance/TaskFailedDependency.tsx:
##
@@ -0,0 +1,70 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from "react";
+import { Text, Table, Tbody, Tr, Th, Td, Box } from "@chakra-ui/react";
+
+import { useTaskFailedDependency } from "src/api";
+
+interface Props {
+ dagId: string;
+ runId: string;
+ taskId: string;
+ mapIndex?: number;
+}
+
+const TaskFailedDependency = ({ dagId, runId, taskId, mapIndex }: Props) => {
+ const { data: dependencies, isLoading } = useTaskFailedDependency({
+dagId,
+taskId,
+runId,
+mapIndex,
+ });
+
+ if (isLoading) {
+return null;
+ }
+
+ return (
+
+
+Task Failed Dependencies
+
+
+
+ Dependencies Blocking Task From Getting Scheduled
+
+
+
+Dependency
+Reason
+
+ {dependencies?.dependencies?.map((dep) => (
Review Comment:
We could also put a spinner or skeleton here during loading. But we may want
to return null if there are no dependencies at all.
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
bbovenzi commented on code in PR #38449:
URL: https://github.com/apache/airflow/pull/38449#discussion_r1550057954
##
airflow/www/static/js/api/useTaskFailedDependency.ts:
##
@@ -0,0 +1,66 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import axios, { AxiosResponse } from "axios";
+import type { API } from "src/types";
+import { useQuery } from "react-query";
+import { getMetaValue } from "../utils";
+
+const taskDependencyURI = getMetaValue("task_dependency_api");
+const mappedTaskDependencyURI = getMetaValue("mapped_task_dependency_api");
+
+export default function useTaskFailedDependency({
+ dagId,
+ taskId,
+ runId,
+ mapIndex,
+}: {
+ dagId: string;
+ taskId: string;
+ runId: string;
+ mapIndex?: number | undefined;
+}) {
+ return useQuery(
+["taskFailedDependencies", dagId, taskId, runId, mapIndex],
+async () => {
+ const definedMapIndex = mapIndex ?? -1;
+ const url = (
+definedMapIndex >= 0 ? mappedTaskDependencyURI : taskDependencyURI
+ )
+.replace("_DAG_RUN_ID_", runId)
+.replace(
+ "_TASK_ID_/0/dependencies",
+ `_TASK_ID_/${mapIndex}/dependencies`
+)
+.replace("_TASK_ID_", taskId);
+
+ try {
Review Comment:
We don't need this try-catch block. useQuery handles errors already.
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
tirkarthi commented on PR #38449: URL: https://github.com/apache/airflow/pull/38449#issuecomment-2020814815 IMO, making this check for None state also helps in catching issues like depends_on_past=True, execution date is in future etc. where task doesn't get to scheduled state. After the separate endpoint I also realized the implementation has more code than expected initially. I tried timing the check and it comes around 2-3 milliseconds with 10 dags so it's not as costly as I thought from getting dagbag from airflow app but just had the session based subtle issues. So I am open to keeping this as additional field to the existing endpoint like first commit or as a separate endpoint or check to be done only when passing a query parameter to existing task instance endpoint like check_dependencies=True.  Thanks -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
bbovenzi commented on PR #38449: URL: https://github.com/apache/airflow/pull/38449#issuecomment-2018275588 > Also the consideration that task_failed_dep looks like a costly operation that it needs to be done for taskinstance schema which is frequently accessed in the UI through autorefresh and other parts though the check for task_failed_deps done is only for tasks in None and failed state. Also, the task_failed_deps really only matters when the task is in `scheduled` state. So a separate endpoint is fine with me. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
tirkarthi commented on PR #38449: URL: https://github.com/apache/airflow/pull/38449#issuecomment-2017597880 From what I understand some of the methods like `dagbag.get_dag` use `create_session` to get the session which invalidates the session attached to ti that has to be available as marshmallow serializes. I tried using inspect to get the session and pass it along so that it's reused. But this more feels like a workaround. I was also curious if this can become a separate endpoint like extra_links endpoint where similar logic of using dagbag is also done. Also the consideration that task_failed_dep looks like a costly operation that it needs to be done for taskinstance schema which is frequently accessed in the UI through autorefresh and other parts though the check for task_failed_deps done is only for tasks in None and failed state. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add task failed dependencies to details page. [airflow]
tirkarthi commented on PR #38449:
URL: https://github.com/apache/airflow/pull/38449#issuecomment-2017397327
I am getting below error in tests. It seems using
`get_airflow_app().dag_bag.get_dag(ti.dag_id)` causes this error but I am not
sure why.
```
def test_should_respond_200(
self, task_instances, update_extras, payload, expected_ti_count,
username, session
):
self.create_task_instances(
session,
update_extras=update_extras,
task_instances=task_instances,
)
> response = self.client.post(
"/api/v1/dags/~/dagRuns/~/taskInstances/list",
environ_overrides={"REMOTE_USER": username},
json=payload,
)
tests/api_connexion/endpoints/test_task_instance_endpoint.py:910:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _
.env/lib/python3.10/site-packages/werkzeug/test.py:1146: in post
return self.open(*args, **kw)
.env/lib/python3.10/site-packages/flask/testing.py:238: in open
response = super().open(
.env/lib/python3.10/site-packages/werkzeug/test.py:1095: in open
response = self.run_wsgi_app(request.environ, buffered=buffered)
.env/lib/python3.10/site-packages/werkzeug/test.py:962: in run_wsgi_app
rv = run_wsgi_app(self.application, environ, buffered=buffered)
.env/lib/python3.10/site-packages/werkzeug/test.py:1243: in run_wsgi_app
app_rv = app(environ, start_response)
.env/lib/python3.10/site-packages/flask/app.py:2552: in __call__
return self.wsgi_app(environ, start_response)
.env/lib/python3.10/site-packages/flask/app.py:2532: in wsgi_app
response = self.handle_exception(e)
.env/lib/python3.10/site-packages/flask/app.py:2529: in wsgi_app
response = self.full_dispatch_request()
.env/lib/python3.10/site-packages/flask/app.py:1825: in full_dispatch_request
rv = self.handle_user_exception(e)
.env/lib/python3.10/site-packages/flask/app.py:1823: in full_dispatch_request
rv = self.dispatch_request()
.env/lib/python3.10/site-packages/flask/app.py:1799: in dispatch_request
return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
.env/lib/python3.10/site-packages/connexion/decorators/decorator.py:68: in
wrapper
response = function(request)
.env/lib/python3.10/site-packages/connexion/decorators/uri_parsing.py:149:
in wrapper
response = function(request)
.env/lib/python3.10/site-packages/connexion/decorators/validation.py:196: in
wrapper
response = function(request)
.env/lib/python3.10/site-packages/connexion/decorators/response.py:112: in
wrapper
response = function(request)
.env/lib/python3.10/site-packages/connexion/decorators/parameter.py:120: in
wrapper
return function(**kwargs)
airflow/api_connexion/security.py:165: in decorated
return _requires_access(
airflow/api_connexion/security.py:92: in _requires_access
return func(*args, **kwargs)
airflow/utils/session.py:79: in wrapper
return func(*args, session=session, **kwargs)
airflow/api_connexion/endpoints/task_instance_endpoint.py:437: in
get_task_instances_batch
return task_instance_collection_schema.dump(
.env/lib/python3.10/site-packages/marshmallow/schema.py:549: in dump
result = self._serialize(processed_obj, many=many)
.env/lib/python3.10/site-packages/marshmallow/schema.py:517: in _serialize
value = field_obj.serialize(attr_name, obj, accessor=self.get_attribute)
.env/lib/python3.10/site-packages/marshmallow/fields.py:340: in serialize
return self._serialize(value, attr, obj, **kwargs)
.env/lib/python3.10/site-packages/marshmallow/fields.py:774: in _serialize
return [self.inner._serialize(each, attr, obj, **kwargs) for each in
value]
.env/lib/python3.10/site-packages/marshmallow/fields.py:774: in
return [self.inner._serialize(each, attr, obj, **kwargs) for each in
value]
.env/lib/python3.10/site-packages/marshmallow/fields.py:643: in _serialize
return schema.dump(nested_obj, many=many)
.env/lib/python3.10/site-packages/marshmallow/schema.py:549: in dump
result = self._serialize(processed_obj, many=many)
.env/lib/python3.10/site-packages/marshmallow/schema.py:517: in _serialize
value = field_obj.serialize(attr_name, obj, accessor=self.get_attribute)
.env/lib/python3.10/site-packages/marshmallow/fields.py:332: in serialize
value = self.get_value(obj, attr, accessor=accessor)
.env/lib/python3.10/site-packages/marshmallow/fields.py:263: in get_value
return accessor_func(obj, check_key, default)
airflow/api_connexion/schemas/task_instance_schema.py:89: in get_attribute
return get_value(obj[0], attr, default)
.env/lib/python3.10/site
[PR] Add task failed dependencies to details page. [airflow]
tirkarthi opened a new pull request, #38449: URL: https://github.com/apache/airflow/pull/38449 Show task failed dependencies when task is in scheduled or none state. None state helps to capture issues when the execution date is in future, depends_on_past fails with past task instance a failure etc. closes: #38189 related: #38189 Scheduled state :  None state :  -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
