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 2022/07/26 09:51:46 UTC
[GitHub] [airflow] bbovenzi opened a new pull request, #25300: List upstream dataset events
bbovenzi opened a new pull request, #25300:
URL: https://github.com/apache/airflow/pull/25300
For dataset-triggered dag runs, add a list of the upstream dataset events since the last dataset-triggered run.
Also:
- Fix initial sort of the dataset events table from created_at to timestamp
- Reuse table cell components
- Change Dag/run/task links into a single link to the source task instance with the dag id
<img width="1377" alt="Screen Shot 2022-07-26 at 10 15 35 AM" src="https://user-images.githubusercontent.com/4600967/180977005-b873ed17-6887-4a2b-ad22-c90af34216df.png">
<img width="622" alt="Screen Shot 2022-07-26 at 10 15 49 AM" src="https://user-images.githubusercontent.com/4600967/180977030-f5b1fe97-2fdb-4236-b053-7c742acc66fa.png">
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an 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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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
[GitHub] [airflow] ashb commented on a diff in pull request #25300: List upstream dataset events
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25300:
URL: https://github.com/apache/airflow/pull/25300#discussion_r929811032
##########
airflow/www/static/js/dag/details/dagRun/UpstreamEvents.tsx:
##########
@@ -0,0 +1,82 @@
+/*!
+ * 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, { useMemo } from 'react';
+import {
+ Box, Heading, Text,
+} from '@chakra-ui/react';
+
+import {
+ CodeCell, DatasetLink, Table, TaskInstanceLink, TimeCell,
+} from 'src/components/Table';
+import { useUpstreamDatasetEvents } from 'src/api';
+import type { DagRun as DagRunType } from 'src/types';
+
+interface Props {
+ runId: DagRunType['runId'];
+}
+
+const UpstreamEvents = ({ runId }: Props) => {
+ const { data: { datasetEvents }, isLoading } = useUpstreamDatasetEvents({ runId });
+
+ const columns = useMemo(
+ () => [
+ {
+ Header: 'Dataset ID',
+ accessor: 'datasetId',
+ Cell: DatasetLink,
+ },
+ {
+ Header: 'Timestamp',
+ accessor: 'timestamp',
+ Cell: TimeCell,
+ },
+ {
+ Header: 'Source Task Instance',
+ accessor: 'sourceTaskId',
+ Cell: TaskInstanceLink,
+ },
+ {
+ Header: 'Extra',
+ accessor: 'extra',
+ disableSortBy: true,
+ Cell: CodeCell,
+ },
+ ],
+ [],
+ );
+
+ const data = useMemo(
+ () => datasetEvents,
+ [datasetEvents],
+ );
Review Comment:
Ditto-isxh here. This memo seems... odd? Shouldn't the cahing from `useUpstreamDatasetEvents` mean we return the same object and so React VDOM knows there is nothing to do 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on pull request #25300: List upstream dataset events
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on PR #25300:
URL: https://github.com/apache/airflow/pull/25300#issuecomment-1195353739
> I have a problem with where we are showing the "Updates to the upstream datasets since the last dataset-triggered DAG run" info. You have it at the bottom of a DagRun, but that is nothing to do with that particular DagRun but something that should be shown at the DAG level
>
> (I think. I know there is a lot of dicussion and work that has gone on that I've missed in the last two weeks, so let me know if I've missing something)
Maybe I need to update the description. These are the dataset events that triggered this particular run. In another PR, we'll list the datasets and any pending events on the DAG details section
--
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
[GitHub] [airflow] bbovenzi commented on a diff in pull request #25300: List upstream dataset events
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25300:
URL: https://github.com/apache/airflow/pull/25300#discussion_r930099432
##########
airflow/www/static/js/dag/details/dagRun/UpstreamEvents.tsx:
##########
@@ -0,0 +1,82 @@
+/*!
+ * 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, { useMemo } from 'react';
+import {
+ Box, Heading, Text,
+} from '@chakra-ui/react';
+
+import {
+ CodeCell, DatasetLink, Table, TaskInstanceLink, TimeCell,
+} from 'src/components/Table';
+import { useUpstreamDatasetEvents } from 'src/api';
+import type { DagRun as DagRunType } from 'src/types';
+
+interface Props {
+ runId: DagRunType['runId'];
+}
+
+const UpstreamEvents = ({ runId }: Props) => {
+ const { data: { datasetEvents }, isLoading } = useUpstreamDatasetEvents({ runId });
+
+ const columns = useMemo(
+ () => [
+ {
+ Header: 'Dataset ID',
+ accessor: 'datasetId',
+ Cell: DatasetLink,
+ },
+ {
+ Header: 'Timestamp',
+ accessor: 'timestamp',
+ Cell: TimeCell,
+ },
+ {
+ Header: 'Source Task Instance',
+ accessor: 'sourceTaskId',
+ Cell: TaskInstanceLink,
+ },
+ {
+ Header: 'Extra',
+ accessor: 'extra',
+ disableSortBy: true,
+ Cell: CodeCell,
+ },
+ ],
+ [],
+ );
Review Comment:
The documentation for `react-table` does say it must be memoized: https://react-table-v7.tanstack.com/docs/api/useTable#table-options
Even their example does an empty dependency memoization. https://react-table-v7.tanstack.com/docs/examples/basic
Eventually we will update from 7.x to 8.0 and we can clean a lot of this 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi merged pull request #25300: List upstream dataset events
Posted by GitBox <gi...@apache.org>.
bbovenzi merged PR #25300:
URL: https://github.com/apache/airflow/pull/25300
--
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
[GitHub] [airflow] ashb commented on a diff in pull request #25300: List upstream dataset events
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25300:
URL: https://github.com/apache/airflow/pull/25300#discussion_r929809860
##########
airflow/www/static/js/dag/details/dagRun/UpstreamEvents.tsx:
##########
@@ -0,0 +1,82 @@
+/*!
+ * 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, { useMemo } from 'react';
+import {
+ Box, Heading, Text,
+} from '@chakra-ui/react';
+
+import {
+ CodeCell, DatasetLink, Table, TaskInstanceLink, TimeCell,
+} from 'src/components/Table';
+import { useUpstreamDatasetEvents } from 'src/api';
+import type { DagRun as DagRunType } from 'src/types';
+
+interface Props {
+ runId: DagRunType['runId'];
+}
+
+const UpstreamEvents = ({ runId }: Props) => {
+ const { data: { datasetEvents }, isLoading } = useUpstreamDatasetEvents({ runId });
+
+ const columns = useMemo(
+ () => [
+ {
+ Header: 'Dataset ID',
+ accessor: 'datasetId',
+ Cell: DatasetLink,
+ },
+ {
+ Header: 'Timestamp',
+ accessor: 'timestamp',
+ Cell: TimeCell,
+ },
+ {
+ Header: 'Source Task Instance',
+ accessor: 'sourceTaskId',
+ Cell: TaskInstanceLink,
+ },
+ {
+ Header: 'Extra',
+ accessor: 'extra',
+ disableSortBy: true,
+ Cell: CodeCell,
+ },
+ ],
+ [],
+ );
Review Comment:
Given this is 100% static why can't it just be a static variable?
--
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
[GitHub] [airflow] ashb commented on pull request #25300: List upstream dataset events
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #25300:
URL: https://github.com/apache/airflow/pull/25300#issuecomment-1195321773
I have a problem with where we are showing the "Updates to the upstream datasets since the last dataset-triggered DAG run" info. You have it at the bottom of a DagRun, but that is nothing to do with that particular DagRun but something that should be shown at the DAG level
(I think. I know there is a lot of dicussion and work that has gone on that I've missed in the last two weeks, so let me know if I've missing something)
--
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
[GitHub] [airflow] bbovenzi commented on a diff in pull request #25300: List upstream dataset events
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25300:
URL: https://github.com/apache/airflow/pull/25300#discussion_r929840556
##########
airflow/www/static/js/dag/details/dagRun/UpstreamEvents.tsx:
##########
@@ -0,0 +1,82 @@
+/*!
+ * 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, { useMemo } from 'react';
+import {
+ Box, Heading, Text,
+} from '@chakra-ui/react';
+
+import {
+ CodeCell, DatasetLink, Table, TaskInstanceLink, TimeCell,
+} from 'src/components/Table';
+import { useUpstreamDatasetEvents } from 'src/api';
+import type { DagRun as DagRunType } from 'src/types';
+
+interface Props {
+ runId: DagRunType['runId'];
+}
+
+const UpstreamEvents = ({ runId }: Props) => {
+ const { data: { datasetEvents }, isLoading } = useUpstreamDatasetEvents({ runId });
+
+ const columns = useMemo(
+ () => [
+ {
+ Header: 'Dataset ID',
+ accessor: 'datasetId',
+ Cell: DatasetLink,
+ },
+ {
+ Header: 'Timestamp',
+ accessor: 'timestamp',
+ Cell: TimeCell,
+ },
+ {
+ Header: 'Source Task Instance',
+ accessor: 'sourceTaskId',
+ Cell: TaskInstanceLink,
+ },
+ {
+ Header: 'Extra',
+ accessor: 'extra',
+ disableSortBy: true,
+ Cell: CodeCell,
+ },
+ ],
+ [],
+ );
Review Comment:
Yeah, i can fix that.
--
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