Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui): Add support for structured reporting of warnings and failures in the UI ingestion flow (ingest uplift 2/2) #10790

Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React from 'react';
import styled from 'styled-components/macro';
import { StyledTable } from '../../entity/shared/components/styled/StyledTable';
import { ANTD_GRAY } from '../../entity/shared/constants';
import { CLI_EXECUTOR_ID } from './utils';
import { CLI_EXECUTOR_ID, getIngestionSourceStatus } from './utils';
import {
LastStatusColumn,
TypeColumn,
Expand Down Expand Up @@ -123,7 +123,7 @@ function IngestionSourceTable({
lastExecStatus:
source.executions &&
source.executions?.executionRequests.length > 0 &&
source.executions?.executionRequests[0].result?.status,
getIngestionSourceStatus(source.executions?.executionRequests[0].result),
cliIngestion: source.config?.executorId === CLI_EXECUTOR_ID,
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ import {
getExecutionRequestStatusDisplayText,
getExecutionRequestStatusIcon,
getExecutionRequestSummaryText,
getIngestionSourceStatus,
getStructuredReport,
RUNNING,
SUCCESS,
} from '../utils';
import { ExecutionRequestResult } from '../../../../types.generated';
import { StructuredReport } from './reporting/StructuredReport';

const StyledTitle = styled(Typography.Title)`
padding: 0px;
Expand Down Expand Up @@ -125,26 +129,30 @@ export const ExecutionDetailsModal = ({ urn, visible, onClose }: Props) => {
};

const logs = (showExpandedLogs && output) || output?.split('\n').slice(0, 5).join('\n');
const result = data?.executionRequest?.result?.status;
const result = data?.executionRequest?.result as Partial<ExecutionRequestResult>;
const status = getIngestionSourceStatus(result);

useEffect(() => {
const interval = setInterval(() => {
if (result === RUNNING) refetch();
if (status === RUNNING) refetch();
}, 2000);

return () => clearInterval(interval);
});

const ResultIcon = result && getExecutionRequestStatusIcon(result);
const resultColor = result && getExecutionRequestStatusDisplayColor(result);
const resultText = result && (
const ResultIcon = status && getExecutionRequestStatusIcon(status);
const resultColor = status && getExecutionRequestStatusDisplayColor(status);
const resultText = status && (
<Typography.Text style={{ color: resultColor, fontSize: 14 }}>
{ResultIcon && <ResultIcon style={{ marginRight: 4 }} />}
{getExecutionRequestStatusDisplayText(result)}
{getExecutionRequestStatusDisplayText(status)}
</Typography.Text>
);

const structuredReport = result && getStructuredReport(result);

const resultSummaryText =
(result && <Typography.Text type="secondary">{getExecutionRequestSummaryText(result)}</Typography.Text>) ||
(status && <Typography.Text type="secondary">{getExecutionRequestSummaryText(status)}</Typography.Text>) ||
undefined;

const recipeJson = data?.executionRequest?.input.arguments?.find((arg) => arg.key === 'recipe')?.value;
Expand All @@ -167,21 +175,22 @@ export const ExecutionDetailsModal = ({ urn, visible, onClose }: Props) => {
bodyStyle={modalBodyStyle}
title={
<HeaderSection>
<StyledTitle level={4}>Ingestion Run Details</StyledTitle>
<StyledTitle level={4}>Sync Details</StyledTitle>
</HeaderSection>
}
visible={visible}
onCancel={onClose}
>
{!data && loading && <Message type="loading" content="Loading execution details..." />}
{error && message.error('Failed to load execution details :(')}
{!data && loading && <Message type="loading" content="Loading sync details..." />}
{error && message.error('Failed to load sync details :(')}
<Section>
<StatusSection>
<Typography.Title level={5}>Status</Typography.Title>
<ResultText>{resultText}</ResultText>
<SubHeaderParagraph>{resultSummaryText}</SubHeaderParagraph>
{structuredReport ? <StructuredReport report={structuredReport} /> : null}
</StatusSection>
{result === SUCCESS && (
{status === SUCCESS && (
<IngestedAssetsSection>
{data?.executionRequest?.id && <IngestedAssets id={data?.executionRequest?.id} />}
</IngestedAssetsSection>
Expand All @@ -190,7 +199,7 @@ export const ExecutionDetailsModal = ({ urn, visible, onClose }: Props) => {
<SectionHeader level={5}>Logs</SectionHeader>
<SectionSubHeader>
<SubHeaderParagraph type="secondary">
View logs that were collected during the ingestion run.
View logs that were collected during the sync.
</SubHeaderParagraph>
<Button type="text" onClick={downloadLogs}>
<DownloadOutlined />
Expand All @@ -213,7 +222,7 @@ export const ExecutionDetailsModal = ({ urn, visible, onClose }: Props) => {
<SectionHeader level={5}>Recipe</SectionHeader>
<SectionSubHeader>
<SubHeaderParagraph type="secondary">
The recipe used for this ingestion run.
The configurations used for this sync with the data source.
</SubHeaderParagraph>
</SectionSubHeader>
<DetailsContainer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import styled from 'styled-components';
import { StyledTable } from '../../../entity/shared/components/styled/StyledTable';
import { ExecutionRequest } from '../../../../types.generated';
import { ButtonsColumn, SourceColumn, StatusColumn, TimeColumn } from './IngestionExecutionTableColumns';
import { SUCCESS } from '../utils';
import { SUCCESS, getIngestionSourceStatus } from '../utils';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

Add test cases for getIngestionSourceStatus.

The function getIngestionSourceStatus is used in multiple places in the codebase, but there are no test cases found for it. To ensure reliability and maintainability, please add comprehensive test cases for this function.

  • Files using getIngestionSourceStatus:
    • datahub-web-react/src/app/ingest/source/IngestionSourceTable.tsx
    • datahub-web-react/src/app/ingest/source/executions/IngestionExecutionTable.tsx
    • datahub-web-react/src/app/ingest/source/executions/ExecutionRequestDetailsModal.tsx
    • datahub-web-react/src/app/ingest/source/utils.ts
Analysis chain

Refactor to use getIngestionSourceStatus for status determination.

The change centralizes the logic for determining the status of ingestion sources, which is a good practice for maintainability. Ensure that the getIngestionSourceStatus function is thoroughly tested to handle all possible statuses correctly.

Also applies to: 115-115

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the correct usage and test coverage of `getIngestionSourceStatus`.

# Test: Search for the test cases for `getIngestionSourceStatus`.
rg --type javascript "getIngestionSourceStatus"

Length of output: 82



Script:

#!/bin/bash
# Description: Verify the correct usage and test coverage of `getIngestionSourceStatus`.

# Search for the usage of `getIngestionSourceStatus` in TypeScript and TSX files.
rg --type-add 'tsx:*.{ts,tsx}' "getIngestionSourceStatus"

# Search for test cases for `getIngestionSourceStatus` in TypeScript and TSX test files.
rg --type-add 'tsx:*.{ts,tsx}' "getIngestionSourceStatus" --glob '*.{test,spec}.{ts,tsx}'

Length of output: 1107

import { formatDuration } from '../../../shared/formatDuration';
import { SearchCfg } from '../../../../conf';

Expand Down Expand Up @@ -112,7 +112,7 @@ export default function IngestionExecutionTable({
requestedAt: execution.input?.requestedAt,
executedAt: execution.result?.startTimeMs,
duration: execution.result?.durationMs,
status: execution.result?.status,
status: getIngestionSourceStatus(execution.result),
showRollback: mostRecentSuccessfulExecution && execution?.urn === mostRecentSuccessfulExecution?.urn,
}));

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import React from 'react';
import styled from 'styled-components';
import { CloseCircleOutlined, ExclamationCircleOutlined, InfoCircleOutlined } from '@ant-design/icons';

import { StructuredReportItemLevel, StructuredReport as StructuredReportType } from '../../types';
import { StructuredReportItemList } from './StructuredReportItemList';
import { REDESIGN_COLORS } from '../../../../entity/shared/constants';

const Container = styled.div`
display: flex;
flex-direction: column;
gap: 12px;
`;

const ERROR_COLOR = '#F5222D';
const WARNING_COLOR = '#FA8C16';
const INFO_COLOR = REDESIGN_COLORS.BLUE;

interface Props {
report: StructuredReportType;
}

export function StructuredReport({ report }: Props) {
if (!report.items.length) {
return null;
}

const warnings = report.items.filter((item) => item.level === StructuredReportItemLevel.WARN);
const errors = report.items.filter((item) => item.level === StructuredReportItemLevel.ERROR);
const infos = report.items.filter((item) => item.level === StructuredReportItemLevel.INFO);
return (
<Container>
{errors.length ? (
<StructuredReportItemList items={errors} color={ERROR_COLOR} icon={CloseCircleOutlined} />
) : null}
{warnings.length ? (
<StructuredReportItemList items={warnings} color={WARNING_COLOR} icon={ExclamationCircleOutlined} />
) : null}
{infos.length ? (
<StructuredReportItemList items={infos} color={INFO_COLOR} icon={InfoCircleOutlined} />
) : null}
</Container>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import React from 'react';
import styled from 'styled-components';
import { Collapse } from 'antd';

import { StructuredReportItem as StructuredReportItemType } from '../../types';
import { ANTD_GRAY } from '../../../../entity/shared/constants';
import { applyOpacity } from '../../../../shared/styleUtils';
import { StructuredReportItemContext } from './StructuredReportItemContext';

const StyledCollapse = styled(Collapse)<{ color: string }>`
background-color: ${(props) => applyOpacity(props.color, 8)};
border: 1px solid ${(props) => applyOpacity(props.color, 20)};
display: flex;

&& {
.ant-collapse-header {
display: flex;
align-items: center;
}

.ant-collapse-item {
border: none;
width: 100%;
}
}
`;

const Item = styled.div`
display: flex;
align-items: center;
justify-content: start;
gap: 4px;
`;

const Content = styled.div`
border-radius: 8px;
`;

const Text = styled.div`
display: flex;
flex-direction: column;
`;

const Type = styled.div`
font-weight: bold;
font-size: 14px;
`;

const Message = styled.div`
color: ${ANTD_GRAY[8]};
`;

interface Props {
item: StructuredReportItemType;
color: string;
icon?: React.ComponentType<any>;
}

export function StructuredReportItem({ item, color, icon }: Props) {
const Icon = icon;
return (
<StyledCollapse color={color}>
<Collapse.Panel
header={
<Item>
{Icon ? <Icon style={{ fontSize: 16, color, marginRight: 12 }} /> : null}
<Text>
<Type>{item.title}</Type>
<Message>{item.message}</Message>
</Text>
</Item>
}
key="0"
>
<Content>
<StructuredReportItemContext item={item} />
</Content>
</Collapse.Panel>
</StyledCollapse>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import React from 'react';
import styled from 'styled-components';
import { Tooltip } from 'antd';

import { StructuredReportItem as StructuredReportItemType } from '../../types';
import { ANTD_GRAY } from '../../../../entity/shared/constants';

const Container = styled.div`
display: flex;
flex-direction: column;
gap: 8px;
margin-left: 12px;
`;

const Title = styled.div`
font-size: 14px;
font-weight: bold;
`;

const Item = styled.div`
padding: 6px;
font-size: 12px;
border-radius: 2px;
background-color: ${ANTD_GRAY[3]};
color: ${ANTD_GRAY[8]};
`;

interface Props {
item: StructuredReportItemType;
}

export function StructuredReportItemContext({ item }: Props) {
return (
<Container>
<Tooltip showArrow={false} title="Additional context about the source of the issue" placement="left">
<Title>Context</Title>
</Tooltip>
{item.context.length
? item.context.map((contextItem) => <Item>{contextItem}</Item>)
: 'No additional context found.'}
</Container>
);
jjoyce0510 marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import React, { useState } from 'react';
import styled from 'styled-components';
import { StructuredReportItem as StructuredReportItemType } from '../../types';
import { StructuredReportItem } from './StructuredReportItem';
import { ShowMoreSection } from '../../../../shared/ShowMoreSection';

const ItemList = styled.div`
display: flex;
flex-direction: column;
gap: 12px;
`;

interface Props {
items: StructuredReportItemType[];
color: string;
icon?: React.ComponentType<any>;
pageSize?: number;
}

export function StructuredReportItemList({ items, color, icon, pageSize = 3 }: Props) {
const [visibleCount, setVisibleCount] = useState(pageSize);
const visibleItems = items.slice(0, visibleCount);
const totalCount = items.length;

return (
<>
<ItemList>
{visibleItems.map((item) => (
<StructuredReportItem item={item} color={color} icon={icon} key={item.rawType} />
))}
</ItemList>
{totalCount > visibleCount ? (
<ShowMoreSection
totalCount={totalCount}
visibleCount={visibleCount}
setVisibleCount={setVisibleCount}
pageSize={pageSize}
/>
) : null}
</>
);
}
Loading
Loading