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/23 23:40:22 UTC
[GitHub] [airflow] dstandish opened a new pull request, #25258: WIP - add table to dagrun details page
dstandish opened a new pull request, #25258:
URL: https://github.com/apache/airflow/pull/25258
Before:
<img width="769" alt="image" src="https://user-images.githubusercontent.com/15932138/180626192-a4b4bd7c-91d0-4312-bb37-2f0b076cb4f8.png">
After:
<img width="778" alt="image" src="https://user-images.githubusercontent.com/15932138/180626238-235f81ba-8d76-4e01-9858-62e53d79d6c3.png">
--
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] dstandish merged pull request #25258: Use tables in grid details panes
Posted by GitBox <gi...@apache.org>.
dstandish merged PR #25258:
URL: https://github.com/apache/airflow/pull/25258
--
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] dstandish commented on pull request #25258: Use tables in grid details panes
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #25258:
URL: https://github.com/apache/airflow/pull/25258#issuecomment-1194064435
btw, to get status icon + status label in same line i had to wrap in a Flex... not sure why but it worked 🤷
--
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 #25258: Use tables in grid details panes
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25258:
URL: https://github.com/apache/airflow/pull/25258#discussion_r928666904
##########
airflow/www/static/js/dag/details/taskInstance/Details.tsx:
##########
@@ -21,7 +21,7 @@ import React from 'react';
import {
Text,
Box,
- Flex,
+ Flex, Table, Tbody, Tr, Td, Heading,
Review Comment:
```suggestion
Flex,
Table,
Tbody,
Tr,
Td,
Heading,
```
--
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 #25258: Use tables in grid details panes
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on PR #25258:
URL: https://github.com/apache/airflow/pull/25258#issuecomment-1193811426
Good idea. That is a lot more consistent from dag -> run -> task. My initial reactions:
- I know I had a header in the DAG details, but seeing it for a run and task instance. It takes up space and maybe we should get rid of all of them
- The status is awkwardly removed from everything else. We should probably move that to the table too?
--
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] dstandish commented on a diff in pull request #25258: Use tables in grid details panes
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #25258:
URL: https://github.com/apache/airflow/pull/25258#discussion_r928871405
##########
airflow/www/static/js/dag/details/dagRun/index.tsx:
##########
@@ -74,85 +74,100 @@ const DagRun = ({ runId }: Props) => {
const detailsLink = appendSearchParams(dagRunDetailsUrl, detailsParams);
return (
- <Box py="4px">
- <Flex justifyContent="space-between" alignItems="center">
- <Button as={Link} variant="ghost" colorScheme="blue" href={detailsLink}>DAG Run Details</Button>
- <Button as={Link} variant="ghost" colorScheme="blue" href={graphLink} leftIcon={<MdOutlineAccountTree />}>
- Graph
- </Button>
- <MarkFailedRun dagId={dagId} runId={runId} />
- <MarkSuccessRun dagId={dagId} runId={runId} />
- </Flex>
- <Divider my={3} />
- <Flex justifyContent="flex-end" alignItems="center">
- <Text fontWeight="bold" mr={2}>Re-run:</Text>
- <ClearRun dagId={dagId} runId={runId} />
- <QueueRun dagId={dagId} runId={runId} />
- </Flex>
- <Divider my={3} />
- <Flex alignItems="center">
- <Text as="strong">Status:</Text>
- <SimpleStatus state={state} mx={2} />
- {state || 'no status'}
- </Flex>
- <br />
- <Text whiteSpace="nowrap">
- Run Id:
- {' '}
- <ClipboardText value={runId} />
- </Text>
- <Text>
- Run Type:
- {' '}
- {runType === 'manual' && <MdPlayArrow style={{ display: 'inline' }} />}
- {runType === 'backfill' && <RiArrowGoBackFill style={{ display: 'inline' }} />}
- {runType === 'scheduled' && <MdOutlineSchedule style={{ display: 'inline' }} />}
- {runType}
- </Text>
- <Text>
- Duration:
- {' '}
- {formatDuration(getDuration(startDate, endDate))}
- </Text>
- {lastSchedulingDecision && (
- <Text>
- Last Scheduling Decision:
- {' '}
- <Time dateTime={lastSchedulingDecision} />
- </Text>
- )}
- <br />
- {startDate && (
- <Text>
- Started:
- {' '}
- <Time dateTime={startDate} />
- </Text>
- )}
- {endDate && (
- <Text>
- Ended:
- {' '}
- <Time dateTime={endDate} />
- </Text>
- )}
- {dataIntervalStart && dataIntervalEnd && (
- <>
- <br />
- <Text as="strong">Data Interval:</Text>
- <Text>
- Start:
- {' '}
- <Time dateTime={dataIntervalStart} />
- </Text>
- <Text>
- End:
- {' '}
- <Time dateTime={dataIntervalEnd} />
- </Text>
- </>
- )}
- </Box>
+ <>
+ <Box>
Review Comment:
you mean the one that opens on 78 and closes on 87?
--
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] dstandish commented on pull request #25258: Use tables in grid details panes
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #25258:
URL: https://github.com/apache/airflow/pull/25258#issuecomment-1194063341
ok @bbovenzi removed header and moved status into table (and updated the screenshots)
--
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] potiuk commented on pull request #25258: Use tables in grid details panes
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25258:
URL: https://github.com/apache/airflow/pull/25258#issuecomment-1194072596
> btw, to get status icon + status label in same line i had to wrap in a Flex... not sure why but it worked shrug
Probably one of those:
--
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 #25258: Use tables in grid details panes
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25258:
URL: https://github.com/apache/airflow/pull/25258#discussion_r928666082
##########
airflow/www/static/js/dag/details/dagRun/index.tsx:
##########
@@ -74,85 +74,100 @@ const DagRun = ({ runId }: Props) => {
const detailsLink = appendSearchParams(dagRunDetailsUrl, detailsParams);
return (
- <Box py="4px">
- <Flex justifyContent="space-between" alignItems="center">
- <Button as={Link} variant="ghost" colorScheme="blue" href={detailsLink}>DAG Run Details</Button>
- <Button as={Link} variant="ghost" colorScheme="blue" href={graphLink} leftIcon={<MdOutlineAccountTree />}>
- Graph
- </Button>
- <MarkFailedRun dagId={dagId} runId={runId} />
- <MarkSuccessRun dagId={dagId} runId={runId} />
- </Flex>
- <Divider my={3} />
- <Flex justifyContent="flex-end" alignItems="center">
- <Text fontWeight="bold" mr={2}>Re-run:</Text>
- <ClearRun dagId={dagId} runId={runId} />
- <QueueRun dagId={dagId} runId={runId} />
- </Flex>
- <Divider my={3} />
- <Flex alignItems="center">
- <Text as="strong">Status:</Text>
- <SimpleStatus state={state} mx={2} />
- {state || 'no status'}
- </Flex>
- <br />
- <Text whiteSpace="nowrap">
- Run Id:
- {' '}
- <ClipboardText value={runId} />
- </Text>
- <Text>
- Run Type:
- {' '}
- {runType === 'manual' && <MdPlayArrow style={{ display: 'inline' }} />}
- {runType === 'backfill' && <RiArrowGoBackFill style={{ display: 'inline' }} />}
- {runType === 'scheduled' && <MdOutlineSchedule style={{ display: 'inline' }} />}
- {runType}
- </Text>
- <Text>
- Duration:
- {' '}
- {formatDuration(getDuration(startDate, endDate))}
- </Text>
- {lastSchedulingDecision && (
- <Text>
- Last Scheduling Decision:
- {' '}
- <Time dateTime={lastSchedulingDecision} />
- </Text>
- )}
- <br />
- {startDate && (
- <Text>
- Started:
- {' '}
- <Time dateTime={startDate} />
- </Text>
- )}
- {endDate && (
- <Text>
- Ended:
- {' '}
- <Time dateTime={endDate} />
- </Text>
- )}
- {dataIntervalStart && dataIntervalEnd && (
- <>
- <br />
- <Text as="strong">Data Interval:</Text>
- <Text>
- Start:
- {' '}
- <Time dateTime={dataIntervalStart} />
- </Text>
- <Text>
- End:
- {' '}
- <Time dateTime={dataIntervalEnd} />
- </Text>
- </>
- )}
- </Box>
+ <>
+ <Box>
Review Comment:
I don't think we need this `<Box>` as the Flex wraps all the children 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-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 #25258: Use tables in grid details panes
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #25258:
URL: https://github.com/apache/airflow/pull/25258#discussion_r928665094
##########
airflow/www/static/js/dag/details/dagRun/index.tsx:
##########
@@ -23,7 +23,7 @@ import {
Box,
Button,
Link,
- Divider,
+ Divider, Table, Tbody, Tr, Td, Heading,
Review Comment:
I don't know how the linting didn't catch this but usually we keep all the imports on separate lines.
```suggestion
Divider,
Table,
Tbody,
Tr,
Td,
Heading,
```
--
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