Skip to content

Commit

Permalink
🪟 🐛 Remove "Cancelled" as a connection status, fix Action Required st…
Browse files Browse the repository at this point in the history 
…atus handling (#6994)
  • Loading branch information
@chandlerprall
chandlerprall committed Jun 5, 2023
1 parent 211a272 commit 8444195
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 38 deletions.
41 changes: 23 additions & 18 deletions airbyte-webapp/src/components/connection/ConnectionStatus/useConnectionStatus.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,26 +255,31 @@ describe("isHandleableScheduledConnection", () => {
});

describe("useConnectionStatus", () => {
/* Cases currently failing:
${"most recent sync was successful, breaking schema changes"} | ${ConnectionStatusIndicatorStatus.ActionRequired} | ${ConnectionStatus.inactive} | ${SchemaChange.breaking} | ${ConnectionSyncStatus.ACTIVE} | ${dayjs().subtract(1, "minute").unix()} | ${undefined} | ${undefined}
${"breaking schema changes, sync is outside of 2x frequency"} | ${ConnectionStatusIndicatorStatus.ActionRequired} | ${ConnectionStatus.inactive} | ${SchemaChange.breaking} | ${ConnectionSyncStatus.FAILED} | ${dayjs().subtract(50, "hours").unix()} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"last sync was cancelled, but last successful sync is within 2x frequency"} | ${ConnectionStatusIndicatorStatus.Late} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.CANCELLED} | ${dayjs().subtract(50, "hours").unix()} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
*/
const within1xFrequency = dayjs().subtract(20, "hours").unix();
const within2xFrequency = dayjs().subtract(28, "hours").unix();
const outside2xFrequency = dayjs().subtract(50, "hours").unix();

it.each`
title | expectedConnectionStatus | connectionStatus | schemaChange | connectionSyncStatus | lastSuccessfulSync | scheduleType | scheduleData
${"most recent sync was successful, no schema changes"} | ${ConnectionStatusIndicatorStatus.OnTime} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.ACTIVE} | ${dayjs().subtract(1, "minute").unix()} | ${undefined} | ${undefined}
${"new connection, not scheduled"} | ${ConnectionStatusIndicatorStatus.Pending} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.EMPTY} | ${undefined} | ${undefined} | ${undefined}
${"new connection, scheduled"} | ${ConnectionStatusIndicatorStatus.Pending} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.EMPTY} | ${undefined} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"connection status is failed, no previous success"} | ${ConnectionStatusIndicatorStatus.Error} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.FAILED} | ${undefined} | ${undefined} | ${undefined}
${"connection status is failed, no previous success"} | ${ConnectionStatusIndicatorStatus.OnTrack} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.FAILED} | ${dayjs().subtract(26, "hours").unix()} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"connection status is failed, last previous success was within 2x schedule frequency"} | ${ConnectionStatusIndicatorStatus.Error} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.FAILED} | ${dayjs().subtract(5, "minutes").unix()} | ${"cron"} | ${{ cronExpression: "* * * * *", cronTimeZone: "UTC" }}
${"connection status is failed, last previous success was a while ago, but cron is never late"} | ${ConnectionStatusIndicatorStatus.Error} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.FAILED} | ${dayjs().subtract(50, "hours").unix()} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"connection status is failed, last previous success was outside 2x schedule frequency"} | ${ConnectionStatusIndicatorStatus.OnTrack} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.ACTIVE} | ${dayjs().subtract(28, "hours").unix()} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"last sync was successful, but the next sync hasn't started (within 2x frequency)"} | ${ConnectionStatusIndicatorStatus.Late} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.ACTIVE} | ${dayjs().subtract(52, "hours").unix()} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"last sync was successful, but the next sync hasn't started (outside 2x frequency)"} | ${ConnectionStatusIndicatorStatus.OnTime} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.ACTIVE} | ${dayjs().subtract(52, "hours").unix()} | ${"cron"} | ${{ cronExpression: "* * * * *", cronTimeZone: "UTC" }}
${"last sync was successful, but the next cron-scheduled sync hasn't started (cron is never late)"} | ${ConnectionStatusIndicatorStatus.Cancelled} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.CANCELLED} | ${dayjs().subtract(2, "hours").unix()} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"last sync was cancelled, but last successful sync is within 1x frequency"} | ${ConnectionStatusIndicatorStatus.Cancelled} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.CANCELLED} | ${dayjs().subtract(2, "hours").unix()} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
title | expectedConnectionStatus | connectionStatus | schemaChange | connectionSyncStatus | lastSuccessfulSync | scheduleType | scheduleData
${"most recent sync was successful, no schema changes"} | ${ConnectionStatusIndicatorStatus.OnTime} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.ACTIVE} | ${within1xFrequency} | ${undefined} | ${undefined}
${"most recent sync was successful, breaking schema changes"} | ${ConnectionStatusIndicatorStatus.ActionRequired} | ${ConnectionStatus.inactive} | ${SchemaChange.breaking} | ${ConnectionSyncStatus.ACTIVE} | ${within1xFrequency} | ${undefined} | ${undefined}
${"breaking schema changes, sync is within 1x frequency"} | ${ConnectionStatusIndicatorStatus.ActionRequired} | ${ConnectionStatus.inactive} | ${SchemaChange.breaking} | ${ConnectionSyncStatus.FAILED} | ${within1xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"breaking schema changes, sync is within 2x frequency"} | ${ConnectionStatusIndicatorStatus.ActionRequired} | ${ConnectionStatus.inactive} | ${SchemaChange.breaking} | ${ConnectionSyncStatus.FAILED} | ${within2xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"breaking schema changes, sync is outside of 2x frequency"} | ${ConnectionStatusIndicatorStatus.ActionRequired} | ${ConnectionStatus.inactive} | ${SchemaChange.breaking} | ${ConnectionSyncStatus.FAILED} | ${outside2xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"new connection, not scheduled"} | ${ConnectionStatusIndicatorStatus.Pending} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.EMPTY} | ${undefined} | ${undefined} | ${undefined}
${"new connection, scheduled"} | ${ConnectionStatusIndicatorStatus.Pending} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.EMPTY} | ${undefined} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"connection status is failed, no previous success"} | ${ConnectionStatusIndicatorStatus.Error} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.FAILED} | ${undefined} | ${undefined} | ${undefined}
${"connection status is failed, last previous success was within 1x schedule frequency"} | ${ConnectionStatusIndicatorStatus.OnTrack} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.FAILED} | ${within1xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"connection status is failed, last previous success was within 2x schedule frequency"} | ${ConnectionStatusIndicatorStatus.OnTrack} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.FAILED} | ${within2xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"connection status is failed, last previous success was within 2x schedule frequency (cron)"} | ${ConnectionStatusIndicatorStatus.Error} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.FAILED} | ${within2xFrequency} | ${"cron"} | ${{ cronExpression: "* * * * *", cronTimeZone: "UTC" }}
${"connection status is failed, last previous success was outside 2x schedule frequency"} | ${ConnectionStatusIndicatorStatus.Error} | ${ConnectionStatus.active} | ${SchemaChange.no_change} | ${ConnectionSyncStatus.FAILED} | ${outside2xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"connection status is failed, last previous success was within 2x schedule frequency"} | ${ConnectionStatusIndicatorStatus.OnTrack} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.ACTIVE} | ${within2xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"last sync was successful, but the next sync hasn't started (outside 2x frequency)"} | ${ConnectionStatusIndicatorStatus.Late} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.ACTIVE} | ${outside2xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"last sync was successful, but the next sync hasn't started (outside 2x frequency, cron)"} | ${ConnectionStatusIndicatorStatus.OnTime} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.ACTIVE} | ${outside2xFrequency} | ${"cron"} | ${{ cronExpression: "* * * * *", cronTimeZone: "UTC" }}
${"last sync was cancelled, but the next cron-scheduled sync hasn't started"} | ${ConnectionStatusIndicatorStatus.OnTime} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.CANCELLED} | ${within1xFrequency} | ${"cron"} | ${{ cronExpression: "* * * * *", cronTimeZone: "UTC" }}
${"last sync was cancelled, but last successful sync is within 1x frequency"} | ${ConnectionStatusIndicatorStatus.OnTime} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.CANCELLED} | ${within1xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"last sync was cancelled, but last successful sync is within 2x frequency"} | ${ConnectionStatusIndicatorStatus.OnTrack} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.CANCELLED} | ${within2xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
${"last sync was cancelled, but last successful sync is outside 2x frequency"} | ${ConnectionStatusIndicatorStatus.Late} | ${ConnectionStatus.active} | ${SchemaChange.non_breaking} | ${ConnectionSyncStatus.CANCELLED} | ${outside2xFrequency} | ${"basic"} | ${{ units: 24, timeUnit: "hours" }}
`(
"$title:" +
"\n\treturns $expectedConnectionStatus when" +
Expand Down
13 changes: 4 additions & 9 deletions airbyte-webapp/src/components/connection/ConnectionStatus/useConnectionStatus.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,15 @@ export const useConnectionStatus = () => {
const lateMultiplier = useLateMultiplierExperiment();
const errorMultiplier = useErrorMultiplierExperiment();

// Disabling the schema changes as it's the only actionable error and is per-connection, not stream
const { connection } = useConnectionEditService();
const { hasBreakingSchemaChange } = useSchemaChanges(connection.schemaChange);

const { connectionEnabled, connectionStatus, lastSuccessfulSync } = useConnectionSyncContext();

if (hasBreakingSchemaChange) {
return ConnectionStatusIndicatorStatus.ActionRequired;
}

if (!connectionEnabled) {
return ConnectionStatusIndicatorStatus.Disabled;
}
Expand All @@ -73,14 +76,6 @@ export const useConnectionStatus = () => {
return ConnectionStatusIndicatorStatus.Error;
}

if (hasBreakingSchemaChange && isConnectionLate(connection, lastSuccessfulSync, lateMultiplier)) {
return ConnectionStatusIndicatorStatus.ActionRequired;
}

if (connectionStatus === ConnectionSyncStatus.CANCELLED) {
return ConnectionStatusIndicatorStatus.Cancelled;
}

// The `late` value is based on the `connection.streamCentricUI.late` experiment
if (isConnectionLate(connection, lastSuccessfulSync, lateMultiplier)) {
return ConnectionStatusIndicatorStatus.Late;
Expand Down
4 changes: 0 additions & 4 deletions ...-webapp/src/components/connection/ConnectionStatusIndicator/ConnectionStatusIndicator.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export enum ConnectionStatusIndicatorStatus {
Error = "error",
ActionRequired = "actionRequired",
Disabled = "disabled",
Cancelled = "cancelled",
}

const ICON_BY_STATUS: Readonly<Record<ConnectionStatusIndicatorStatus, JSX.Element>> = {
Expand All @@ -27,7 +26,6 @@ const ICON_BY_STATUS: Readonly<Record<ConnectionStatusIndicatorStatus, JSX.Eleme
pending: <ClockIcon />,
late: <ClockIcon />,
actionRequired: <Icon type="cross" withBackground />,
cancelled: <Icon type="minus" color="action" withBackground />,
};

const STYLE_BY_STATUS: Readonly<Record<ConnectionStatusIndicatorStatus, string>> = {
Expand All @@ -38,7 +36,6 @@ const STYLE_BY_STATUS: Readonly<Record<ConnectionStatusIndicatorStatus, string>>
pending: styles["status--pending"],
late: styles["status--late"],
actionRequired: styles["status--actionRequired"],
cancelled: styles["status--cancelled"],
};

const BOX_STYLE_BY_STATUS: Readonly<Record<ConnectionStatusIndicatorStatus, string>> = {
Expand All @@ -49,7 +46,6 @@ const BOX_STYLE_BY_STATUS: Readonly<Record<ConnectionStatusIndicatorStatus, stri
pending: styles["status--pending-withBox"],
late: styles["status--late-withBox"],
actionRequired: styles["status--actionRequired-withBox"],
cancelled: styles["status--cancelled-withBox"],
};

interface ConnectionStatusIndicatorProps {
Expand Down
7 changes: 1 addition & 6 deletions airbyte-webapp/src/components/connection/ConnectionSync/ConnectionSyncContext.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,7 @@ interface ConnectionSyncContext {
lastSuccessfulSync?: number;
}

const jobStatusesIndicatingFinishedExecution: string[] = [
JobStatus.cancelled,
JobStatus.succeeded,
JobStatus.failed,
JobStatus.incomplete,
];
const jobStatusesIndicatingFinishedExecution: string[] = [JobStatus.succeeded, JobStatus.failed, JobStatus.incomplete];
const useConnectionSyncContextInit = (jobs: JobWithAttemptsRead[]): ConnectionSyncContext => {
const { connection } = useConnectionEditService();
const [activeJob, setActiveJob] = useState(jobs[0]?.job);
Expand Down
1 change: 0 additions & 1 deletion airbyte-webapp/src/pages/connections/StreamStatusPage/ConnectionStatusOverview.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const MESSAGE_BY_STATUS: Readonly<Record<ConnectionStatusIndicatorStatus, string
error: "connection.status.error",
actionRequired: "connection.status.actionRequired",
disabled: "connection.status.disabled",
cancelled: "connection.status.cancelled",
};

export const ConnectionStatusOverview: React.FC = () => {
Expand Down

0 comments on commit 8444195

Please sign in to comment.