Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
106 changes: 55 additions & 51 deletions torchci/components/job/GroupJobConclusion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import {
isGroupAutorevertSignal,
isJobAutorevertSignal,
} from "lib/autorevertUtils";
import { getGroupConclusionChar } from "lib/JobClassifierUtil";
import {
getGroupConclusionChar,
isJobViableStrictBlocking,
} from "lib/JobClassifierUtil";
import {
isCancellationSuccessJob,
isFailedJob,
Expand Down Expand Up @@ -43,44 +46,6 @@ export enum GroupedJobStatus {
Pending = "pending",
}

type RepoViableStrictBlockingJobsMap = {
[key: string]: RegExp[];
};

// TODO: Move this to a config file
const VIABLE_STRICT_BLOCKING_JOBS: RepoViableStrictBlockingJobsMap = {
// Source of truth for these jobs is in https://github.com/pytorch/pytorch/blob/main/.github/workflows/update-viablestrict.yml#L26
"pytorch/pytorch": [
/trunk/i,
/pull/i,
/lint/i,
/linux-binary-libtorch-release /i,
/linux-binary-manywheel /i,
/linux-aarch64/i,
],
};

function isJobViableStrictBlocking(
jobName: string | undefined,
repoOwner: string,
repoName: string
): boolean {
if (!jobName) {
return false;
}

const repo = `${repoOwner}/${repoName}`;
let viablestrict_blocking_jobs_patterns =
VIABLE_STRICT_BLOCKING_JOBS[repo] ?? [];

for (const regex of viablestrict_blocking_jobs_patterns) {
if (jobName.match(regex)) {
return true;
}
}
return false;
}

// React component to render either a group conclusion character or monsterized icons for failures
function GroupConclusionContent({
conclusion,
Expand Down Expand Up @@ -276,6 +241,8 @@ export default function HudGroupedCell({
failedPreviousRunJobs={failedPreviousRunJobs}
sha={sha}
rowData={rowData}
repoOwner={repoOwner}
repoName={repoName}
/>
}
>
Expand Down Expand Up @@ -326,6 +293,8 @@ function GroupTooltip({
failedPreviousRunJobs,
sha,
rowData,
repoOwner,
repoName,
}: {
conclusion: GroupedJobStatus;
groupName: string;
Expand All @@ -335,6 +304,8 @@ function GroupTooltip({
failedPreviousRunJobs: JobData[];
sha?: string;
rowData?: RowData;
repoOwner?: string;
repoName?: string;
}) {
const [monsterFailures] = useContext(MonsterFailuresContext);

Expand Down Expand Up @@ -382,6 +353,10 @@ function GroupTooltip({
const isAutorevert = rowData
? isJobAutorevertSignal(job, rowData)
: false;
const isViableStrictBlocking =
repoOwner &&
repoName &&
isJobViableStrictBlocking(job.name, repoOwner, repoName);
return (
<div
key={jobIndex}
Expand All @@ -400,6 +375,12 @@ function GroupTooltip({
{job.name}
{isAutorevert && " ⚠️ (triggered autorevert)"}
</a>
{isViableStrictBlocking && (
<span style={{ color: "orange", fontWeight: "bold" }}>
{" "}
[viable/strict blocking]
</span>
)}
</div>
);
})}
Expand All @@ -416,6 +397,8 @@ function GroupTooltip({
jobs={erroredJobs}
message={"The following jobs errored out:"}
rowData={rowData}
repoOwner={repoOwner}
repoName={repoName}
/>
);
} else if (conclusion === GroupedJobStatus.Queued) {
Expand All @@ -426,6 +409,8 @@ function GroupTooltip({
jobs={queuedJobs}
message={"The following jobs are still in queue:"}
rowData={rowData}
repoOwner={repoOwner}
repoName={repoName}
/>
);
} else if (conclusion === GroupedJobStatus.Pending) {
Expand All @@ -436,6 +421,8 @@ function GroupTooltip({
jobs={pendingJobs}
message={"The following jobs are still pending:"}
rowData={rowData}
repoOwner={repoOwner}
repoName={repoName}
/>
);
} else if (conclusion === GroupedJobStatus.Flaky) {
Expand All @@ -446,6 +433,8 @@ function GroupTooltip({
jobs={failedPreviousRunJobs}
message={"The following jobs were flaky:"}
rowData={rowData}
repoOwner={repoOwner}
repoName={repoName}
/>
);
} else if (conclusion === GroupedJobStatus.AllNull) {
Expand Down Expand Up @@ -473,12 +462,16 @@ function ToolTip({
message,
jobs,
rowData,
repoOwner,
repoName,
}: {
conclusion: string;
groupName: string;
message: string;
jobs: JobData[];
rowData?: RowData;
repoOwner?: string;
repoName?: string;
}) {
return (
<div>
Expand All @@ -488,19 +481,30 @@ function ToolTip({
const isAutorevert = rowData
? isJobAutorevertSignal(job, rowData)
: false;
const isViableStrictBlocking =
repoOwner &&
repoName &&
isJobViableStrictBlocking(job.name, repoOwner, repoName);
return (
<a
key={ind}
href={job.htmlUrl}
target="_blank"
rel="noreferrer"
className={
isAutorevert ? styles.autorevert_tooltip_anchor : undefined
}
>
{job.name}
{isAutorevert && " ⚠️ (triggered autorevert)"}
</a>
<div key={ind}>
<a
href={job.htmlUrl}
target="_blank"
rel="noreferrer"
className={
isAutorevert ? styles.autorevert_tooltip_anchor : undefined
}
>
{job.name}
{isAutorevert && " ⚠️ (triggered autorevert)"}
</a>
{isViableStrictBlocking && (
<span style={{ color: "orange", fontWeight: "bold" }}>
{" "}
[viable/strict blocking]
</span>
)}
</div>
);
})}
</div>
Expand Down
5 changes: 3 additions & 2 deletions torchci/components/job/JobConclusion.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@
}

.viablestrict_blocking {
border-left: 1px solid var(--color-failure);
border-right: 1px solid var(--color-failure);
border: 2px solid var(--color-failure);
background-color: rgba(255, 0, 0, 0.1);
border-radius: 2px;
}

.autorevert_tooltip_anchor {
Expand Down
15 changes: 15 additions & 0 deletions torchci/components/job/JobTooltip.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isJobViableStrictBlocking } from "lib/JobClassifierUtil";
import { JobData } from "../../lib/types";
import { SingleWorkflowDispatcher } from "../commit/WorkflowDispatcher";
import LogViewer from "../common/log/LogViewer";
Expand All @@ -7,10 +8,14 @@ export default function JobTooltip({
job,
sha,
isAutorevertSignal,
repoOwner,
repoName,
}: {
job: JobData;
sha?: string;
isAutorevertSignal?: boolean;
repoOwner?: string;
repoName?: string;
}) {
// For nonexistent jobs, just show something basic:
if (!job.hasOwnProperty("id")) {
Expand All @@ -24,6 +29,11 @@ export default function JobTooltip({
);
}

const isViableStrictBlocking =
repoOwner &&
repoName &&
isJobViableStrictBlocking(job.name, repoOwner, repoName);

return (
<div>
{`[${job.conclusion}] ${job.name}`}
Expand All @@ -32,6 +42,11 @@ export default function JobTooltip({
Failure in this job has triggered autorevert.
</div>
)}
{isViableStrictBlocking && (
<div style={{ color: "orange", fontWeight: "bold" }}>
This job is viable/strict blocking.
</div>
)}
<div>
<em>click to pin this tooltip, double-click for job page</em>
</div>
Expand Down
61 changes: 60 additions & 1 deletion torchci/lib/JobClassifierUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,41 @@ import { GroupedJobStatus, JobStatus } from "components/job/GroupJobConclusion";
import { getOpenUnstableIssues } from "lib/jobUtils";
import { IssueData, RowData } from "./types";

type RepoViableStrictBlockingJobsMap = {
[key: string]: RegExp[];
};

// Source of truth for these jobs is in https://github.com/pytorch/pytorch/blob/main/.github/workflows/update-viablestrict.yml#L26
export const VIABLE_STRICT_BLOCKING_JOBS: RepoViableStrictBlockingJobsMap = {
"pytorch/pytorch": [/pull/i, /trunk/i, /lint/i, /linux-aarch64/i],
Copy link
Member

Choose a reason for hiding this comment

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

This might be hard to maintain.

I'm curious if we can have this be populated somewhere in clickhouse and we can just pull it from there.

It's fine for a first pass but we should make a TODO / issue about revising it to do the right thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally agree, but it was hardcoded this way before this PR, I just moved it to the different file.

};

export function isJobViableStrictBlocking(
jobName: string | undefined,
repoOwner: string,
repoName: string
): boolean {
if (!jobName) {
return false;
}

// Exclude memory leak and rerun disabled tests jobs
if (jobName.match(/, mem_leak/) || jobName.match(/, rerun_/)) {
return false;
}

const repo = `${repoOwner}/${repoName}`;
const viablestrictBlockingJobsPatterns =
VIABLE_STRICT_BLOCKING_JOBS[repo] ?? [];

for (const regex of viablestrictBlockingJobsPatterns) {
if (jobName.match(regex)) {
return true;
}
}
return false;
}

const GROUP_MEMORY_LEAK_CHECK = "Memory Leak Check";
const GROUP_RERUN_DISABLED_TESTS = "Rerun Disabled Tests";
const GROUP_UNSTABLE = "Unstable";
Expand Down Expand Up @@ -355,14 +390,19 @@ export function getGroupingData(
shaGrid: RowData[],
jobNames: Set<string>,
showUnstableGroup: boolean,
unstableIssues?: IssueData[]
unstableIssues?: IssueData[],
repoOwner?: string,
repoName?: string
) {
// Construct Job Groupping Mapping
const groupNameMapping = new Map<string, Array<string>>(); // group -> [job names]

// Track which jobs have failures
const jobsWithFailures = new Set<string>();

// Track which jobs are viable/strict blocking
const jobsViableStrictBlocking = new Set<string>();

// First pass: check failures for each job across all commits
for (const name of jobNames) {
// Check if this job has failures in any commit
Expand All @@ -374,6 +414,15 @@ export function getGroupingData(
if (hasFailure) {
jobsWithFailures.add(name);
}

// Check if this job is viable/strict blocking
if (
repoOwner &&
repoName &&
isJobViableStrictBlocking(name, repoOwner, repoName)
) {
jobsViableStrictBlocking.add(name);
}
}

// Second pass: group jobs
Expand All @@ -392,11 +441,21 @@ export function getGroupingData(
}
}

// Calculate which groups have viable/strict blocking jobs
const groupsViableStrictBlocking = new Set<string>();
for (const [groupName, jobs] of groupNameMapping.entries()) {
if (jobs.some((jobName) => jobsViableStrictBlocking.has(jobName))) {
groupsViableStrictBlocking.add(groupName);
}
}

return {
shaGrid,
groupNameMapping,
jobsWithFailures,
groupsWithFailures,
jobsViableStrictBlocking,
groupsViableStrictBlocking,
};
}

Expand Down
12 changes: 12 additions & 0 deletions torchci/lib/useGroupingPreference.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,15 @@ export function useHideGreenColumnsPreference(): [
);
return [state, setState];
}

export function useHideNonViableStrictPreference(): [
boolean,
(_hideNonViableStrictValue: boolean) => void
] {
const [state, setState] = usePreference(
"hideNonViableStrict",
/*override*/ undefined,
/*default*/ false
);
return [state, setState];
}
Loading