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.

Sign up for GitHub

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

[retryBot] Repo-specific lists of retriable workflows #3942

Merged
merged 4 commits into from
Mar 29, 2023
Merged

[retryBot] Repo-specific lists of retriable workflows #3942

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b548da9
Use a separate list of allowed workflows per repo
DanilBaibak Mar 28, 2023
8f63c25
Added type
DanilBaibak Mar 28, 2023
74f048c
Fixed test
DanilBaibak Mar 28, 2023
88c33df
Use 'pytorch' as a fallback for repos not covered by the map
DanilBaibak Mar 29, 2023
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
23 changes: 11 additions & 12 deletions torchci/lib/bot/retryBot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ function retryBot(app: Probot): void {
const workflowName = ctx.payload.workflow_run.name;
const attemptNumber = ctx.payload.workflow_run.run_attempt;
const defaultBranch = ctx.payload.repository.default_branch;
const allowedWorkflowPrefixes = [
"lint",
"pull",
"trunk",
"linux-binary",
"windows-binary"
]
const owner = ctx.payload.repository.owner.login;
const repo = ctx.payload.repository.name;
const runId = ctx.payload.workflow_run.id;

const allowedWorkflowPrefixes: { [key: string]: string[] } = {
"pytorch": ["lint", "pull", "trunk", "linux-binary", "windows-binary"],
"vision": ["lint", "Build Linux", "Build Macos", "Build M1", "Tests on Linux", "Tests on macOS"]
Comment on lines +14 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't have to do it in this PR, but it would be great if this could be extracted out to a repo-specific config file that contains each repo's allowed prefixes (with a default fallback list).

Example of a repo-specific config used by the auto-labeler: https://github.com/pytorch/pytorch/blob/master/.github/labeler.yml

Copy link
Contributor

Choose a reason for hiding this comment

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

@ZainRizvi I don't think labelled is used by the probot
But repo-specific config that bot reads is https://github.com/pytorch/pytorch/blob/master/.github/pytorch-probot.yml

Copy link
Contributor

@ZainRizvi ZainRizvi Mar 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I didn't mean to say that specifically the auto-labler config should be used, but to use some repo specific config, similar to it. pytorch-probot.yml would be a good config to use here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is an example, how config is used in ciflow bot:

test-infra/torchci/lib/bot/ciflowPushTrigger.ts

Lines 139 to 148 in a1d729c

const config: any = await tracker.loadConfig(context);
const valid_labels: Array<string> =
config !== null ? config["ciflow_push_tags"] : null;
if (valid_labels == null) {
await context.octokit.issues.createComment(
context.repo({
body: "No ciflow labels are configured for this repo.\n" +
"For information on how to enable CIFlow bot see " +
"this [wiki]( https://github.com/pytorch/test-infra/wiki/PyTorch-bot#ciflow-bot)",
issue_number: context.payload.pull_request.number,

The idea is to create PRs against core and vision that introduces say retriable_workflows string-array property and later modify bot to read those

}
const allowedRepoPrefixes = allowedWorkflowPrefixes[repo] ? allowedWorkflowPrefixes[repo] : allowedWorkflowPrefixes["pytorch"];
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit (two statements are identical in typescript)

Suggested change
const allowedRepoPrefixes = allowedWorkflowPrefixes[repo] ? allowedWorkflowPrefixes[repo] : allowedWorkflowPrefixes["pytorch"];
const allowedRepoPrefixes = allowedWorkflowPrefixes[repo] || allowedWorkflowPrefixes["pytorch"];


if (
ctx.payload.workflow_run.conclusion === "success" ||
Expand All @@ -22,15 +24,12 @@ function retryBot(app: Probot): void {
ctx.payload.workflow_run.head_branch !== defaultBranch
) ||
attemptNumber > 1 ||
allowedWorkflowPrefixes.every(
allowedWorkflow => !workflowName.toLowerCase().includes(allowedWorkflow)
allowedRepoPrefixes.every(
allowedWorkflow => !workflowName.toLowerCase().includes(allowedWorkflow.toLowerCase())
)
) {
return;
}
const owner = ctx.payload.repository.owner.login;
const repo = ctx.payload.repository.name;
const runId = ctx.payload.workflow_run.id;

let workflowJobs = [];
let total_count = 1;
Expand Down