Skip to content

Commit

Permalink
Don't skip waiting if the run was triggered by another job in the sam…
Browse files Browse the repository at this point in the history
…e workflow (#755)

This is a bug, but I bumped the major version for the behavior changes.

Add a new option `skip-same-workflow` to keep the behavior as v2 era.
A workflow that has multiple `wait-other-jobs` patterns appears frequently in my use.
And the `skip-list` can't track file name changes, so adding an option will fit here.

Other changes are chore tasks, refactored, and added a minimum changelog.

Fixes #754
  • Loading branch information
kachick committed Apr 12, 2024
1 parent a1b204a commit 9d1579c
Show file tree
Hide file tree
Showing 12 changed files with 175 additions and 88 deletions.
3 changes: 1 addition & 2 deletions .github/ISSUE_TEMPLATE/BUG-REPORT.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ body:
label: Version
description: What version are you using?
options:
- 'v3.0.0' # selfup { "regex": "v\\d[^']+", "script": "git tag --list 'v3.*' | tail -1" }
- 'v2.0.4' # selfup { "regex": "v\\d[^']+", "script": "git tag --list 'v2.*' | tail -1" }
- 'v2'
- 'v1.3.0' # selfup { "regex": "v\\d[^']+", "script": "git tag --list 'v1.*' | tail -1" }
- 'v1'
- other
default: 0
validations:
Expand Down
34 changes: 23 additions & 11 deletions .github/workflows/itself.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,40 +16,48 @@ on:
# However PRs will have read permissions because this project is on a public repository
permissions: {}

# Do not enable `ACTIONS_STEP_DEBUG: true` for all steps, I also want to check actual logging

jobs:
# Keep sequential to avoid infinite loop
default_logic:
runs-on: ubuntu-latest
if: ${{ github.actor != 'dependabot[bot]' && github.actor != 'renovate[bot]' }}
timeout-minutes: 10
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
# Do NOT specify any options here to make sure zero config may work
- uses: ./
exponential_backoff:
runs-on: ubuntu-latest
if: ${{ github.actor != 'dependabot[bot]' && github.actor != 'renovate[bot]' }}
timeout-minutes: 5
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
- uses: ./
env:
ACTIONS_STEP_DEBUG: true
continue-on-error: true
with:
retry-method: 'exponential_backoff'
min-interval-seconds: 5
# Set low intervals but stop faster. Returning false is an intentional only in this job
wait-seconds-before-first-polling: 2
min-interval-seconds: 2
attempt-limits: 2
skip-same-workflow: 'true'
equal_intervals:
needs: [exponential_backoff]
runs-on: ubuntu-latest
if: ${{ github.actor != 'dependabot[bot]' && github.actor != 'renovate[bot]' }}
timeout-minutes: 10
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
- uses: ./
env:
ACTIONS_STEP_DEBUG: true
with:
retry-method: 'equal_intervals'
min-interval-seconds: 10
attempt-limits: 60
default_logic:
needs: [equal_intervals]
runs-on: ubuntu-latest
if: ${{ github.actor != 'dependabot[bot]' && github.actor != 'renovate[bot]' }}
timeout-minutes: 10
steps:
- uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
- uses: ./
skip-same-workflow: 'true'
wait-list:
runs-on: ubuntu-latest
if: ${{ github.actor != 'dependabot[bot]' && github.actor != 'renovate[bot]' }}
Expand Down Expand Up @@ -82,8 +90,12 @@ jobs:
retry-method: 'equal_intervals'
min-interval-seconds: 5
attempt-limits: 30
skip-same-workflow: 'false' # Intentionally set false to test skip list also can cover this use case
skip-list: |
[
{
"workflowFile": "itself.yml"
},
{
"workflowFile": "ci.yml"
},
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/merge-bot-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ jobs:
- name: Wait other jobs
uses: ./
timeout-minutes: 10
with:
skip-same-workflow: 'true'
- name: Approve and merge
run: gh pr review --approve "$PR_URL" && gh pr merge --auto --squash --delete-branch "$PR_URL"
env:
Expand All @@ -61,6 +63,7 @@ jobs:
with:
retry-method: 'equal_intervals'
min-interval-seconds: '15'
skip-same-workflow: 'true'
- name: Approve and merge
run: gh pr review --approve "$PR_URL" && gh pr merge --auto --squash --delete-branch "$PR_URL"
env:
Expand All @@ -78,6 +81,8 @@ jobs:
- name: Wait other jobs
uses: ./
timeout-minutes: 20
with:
skip-same-workflow: 'true'
- name: Approve and merge
run: gh pr review --approve "$PR_URL" && gh pr merge --auto --delete-branch --squash "$PR_URL"
env:
Expand Down
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# CHANGELOG

This file only records notable changes. Not synchronized with all releases and tags.

- v3.0.0
- Wait other jobs which defined in same workflow by default: #754\
You can change this behavior with new option `skip-same-workflow: 'true'`
- v2.0.2
- Allow some neutral patterns: 93299c2fa22fd463db31668eba54b34b58270696
- v2.0.0
- Add options for list-based waiting with replacing to GraphQL API: #474, #574
- Update action engine to Node20: #586, #564
- Change default intervals and the determined logics: #596, 15456c0
- v1.3.0
- Provide default value for `github-token`: #523
40 changes: 23 additions & 17 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ jobs:
# actions: read
runs-on: ubuntu-latest
steps:
- uses: kachick/wait-other-jobs@v2
# timeout-minutes: 15 # Recommended to be enabled with reasonable value
- uses: kachick/wait-other-jobs@v3.0.0
timeout-minutes: 15 # Recommended to be enabled with your appropriate value for fail-safe use
```

You can change the token, polling interval, allow/deny list and turns early-exit as below.
Expand All @@ -33,6 +33,7 @@ with:
min-interval-seconds: '300' # default '15'
retry-method: 'exponential_backoff' # default 'equal_intervals'
early-exit: 'false' # default 'true'
skip-same-workflow: 'true' # default 'false'
# lists should be given with JSON formatted array, do not specify both wait-list and skip-list
# - Each items should have "workflowFile" field and they can optinaly have "jobName" field
# - If no jobName is specified, all of jobs in the workflow will be targeted
Expand All @@ -56,17 +57,18 @@ with:

Full list of the options

| NAME | DESCRIPTION | TYPE | REQUIRED | DEFAULT | OPTIONS |
| ----------------------------------- | ------------------------------------------------------------------------------- | -------- | -------- | --------------------- | ---------------------------------------- |
| `github-token` | The GITHUB_TOKEN secret. You can use PAT if you want. | `string` | `true` | `${{ github.token }}` | |
| `wait-seconds-before-first-polling` | Wait this interval before first polling | `number` | `false` | `10` | |
| `min-interval-seconds` | Wait this interval or the multiplied value (and jitter) for next polling | `number` | `false` | `15` | |
| `retry-method` | How to wait for next polling | `string` | `false` | `equal_intervals` | `exponential_backoff`, `equal_intervals` |
| `early-exit` | Stop rest pollings if faced at least 1 bad condition | `bool` | `false` | `true` | |
| `attempt-limits` | Stop rest pollings after this attempts even if other jobs are not yet completed | `number` | `false` | `1000` | |
| `wait-list` | This action will not wait for items other than this list | `string` | `false` | `[]` | |
| `skip-list` | This action will not wait for items on this list | `string` | `false` | `[]` | |
| `dry-run` | Avoid requests for tests | `bool` | `false` | `false` | |
| NAME | DESCRIPTION | TYPE | DEFAULT | OPTIONS |
| ----------------------------------- | -------------------------------------------------------------- | -------- | --------------------- | ---------------------------------------- |
| `github-token` | The GITHUB_TOKEN secret. You can use PAT if you want. | `string` | `${{ github.token }}` | |
| `wait-seconds-before-first-polling` | Wait this interval before first polling | `number` | `10` | |
| `min-interval-seconds` | Wait this interval or the multiplied value (and jitter) | `number` | `15` | |
| `retry-method` | How to wait for next polling | `string` | `equal_intervals` | `exponential_backoff`, `equal_intervals` |
| `early-exit` | Stop rest pollings if faced at least 1 bad condition | `bool` | `true` | |
| `attempt-limits` | Stop rest pollings if reached to this limit | `number` | `1000` | |
| `wait-list` | Wait only these jobs | `string` | `[]` | |
| `skip-list` | Wait except these jobs | `string` | `[]` | |
| `skip-same-workflow` | Skip jobs defined in the same workflow which using this action | `bool` | `false` | |
| `dry-run` | Avoid requests for tests | `bool` | `false` | |

## Required GTHUB_TOKEN permissions

Expand All @@ -86,11 +88,15 @@ See the [docs](docs/examples.md) for further detail.

## Limitations

Judge OK or Bad with the checkRun state at the moment.\
When some jobs will be triggered after this action with `needs: [distant-first]`, it might be unaccurate. (I didn't faced yet)
- If you use this action in multiple jobs on the same repository, you should avoid deadlocks.\
The `skip-list`, `wait-list` and `skip-same-workflow` options cover this use case.

If any workflow starts many jobs as 100+, this action does not support it.\
Because of nested paging in GraphQL makes complex. See [related docs](https://github.com/octokit/plugin-paginate-graphql.js/blob/a6b12e867466b0c583b002acd1cb1ed90b11841f/README.md#L184-L218) for further detail.
- Judge OK or Bad with the checkRun state at the moment.\
When some jobs will be triggered after this action with `needs: [distant-first]`, it might be unaccurate.\
(I didn't see actual example yet)

- If any workflow starts many jobs as 100+, this action does not support it.\
Because of nested paging in GraphQL makes complex. See [related docs](https://github.com/octokit/plugin-paginate-graphql.js/blob/a6b12e867466b0c583b002acd1cb1ed90b11841f/README.md#L184-L218) for further detail.

## License

Expand Down
14 changes: 9 additions & 5 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ branding:
inputs:
github-token:
description: 'The GITHUB_TOKEN secret'
required: true
required: false
default: ${{ github.token }}
wait-seconds-before-first-polling:
description: 'Wait this seconds before first polling'
required: false
default: '10'
min-interval-seconds:
description: 'Wait this interval or the multiplied value (and jitter) for next polling'
description: 'Wait this interval or the multiplied value (and jitter)'
required: false
default: '15'
early-exit:
Expand All @@ -26,17 +26,21 @@ inputs:
required: false
default: 'equal_intervals'
attempt-limits:
description: 'Stop rest pollings after this limits even if other jobs are not yet completed'
description: 'Stop rest pollings if reached to this limit'
required: false
default: '1000' # Enough large
wait-list:
description: 'This action will not wait for items other than this list'
description: 'Wait only these jobs'
required: false
default: '[]'
skip-list:
description: 'This action will not wait for items on this list'
description: 'Wait except these jobs'
required: false
default: '[]'
skip-same-workflow:
description: 'Skip jobs defined in the same workflow which using this action'
required: false
default: 'false'
dry-run:
description: 'Avoid http requests for tests'
required: false
Expand Down
2 changes: 1 addition & 1 deletion deno.jsonc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"lint": {
"include": ["."],
"exclude": ["dist/", "node_modules/"],
"exclude": ["dist/", "node_modules/", "logs/"],
"rules": {
"tags": ["recommended"]
}
Expand Down
61 changes: 38 additions & 23 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28144,12 +28144,12 @@ var z = /* @__PURE__ */ Object.freeze({

// src/github-api.ts
var PaginatableOctokit = Octokit.plugin(paginateGraphQL);
var ListItem = z.object({
var FilterCondition = z.object({
workflowFile: z.string().endsWith(".yml"),
jobName: z.string().min(1).optional()
});
var List = z.array(ListItem);
async function getCheckRunSummaries(token, params) {
var FilterConditions = z.array(FilterCondition);
async function getCheckRunSummaries(token, trigger) {
const octokit = new PaginatableOctokit({ auth: token });
const { repository: { object: { checkSuites } } } = await octokit.graphql.paginate(
`
Expand Down Expand Up @@ -28195,9 +28195,9 @@ async function getCheckRunSummaries(token, params) {
}
`,
{
owner: params.owner,
repo: params.repo,
commitSha: params.ref
owner: trigger.owner,
repo: trigger.repo,
commitSha: trigger.ref
}
);
const checkSuiteNodes = checkSuites?.nodes?.flatMap((node) => node ? [node] : []);
Expand All @@ -28209,9 +28209,6 @@ async function getCheckRunSummaries(token, params) {
if (!workflow) {
return [];
}
if (checkSuite.workflowRun?.databaseId === params.triggerRunId) {
return [];
}
const checkRuns = checkSuite?.checkRuns;
if (!checkRuns) {
throw new Error("no checkRuns");
Expand All @@ -28225,7 +28222,9 @@ async function getCheckRunSummaries(token, params) {
}
return runNodes.map((run2) => ({
acceptable: run2.conclusion == "SUCCESS" || run2.conclusion === "SKIPPED" || run2.conclusion === "NEUTRAL" && (checkSuite.conclusion === "SUCCESS" || checkSuite.conclusion === "SKIPPED"),
workflowPath: relative(`/${params.owner}/${params.repo}/actions/workflows/`, workflow.resourcePath),
workflowPath: relative(`/${trigger.owner}/${trigger.repo}/actions/workflows/`, workflow.resourcePath),
// Another file can set same workflow name. So you should filter workfrows from runId or the filename
isSameWorkflow: checkSuite.workflowRun?.databaseId === trigger.runId,
checkSuiteStatus: checkSuite.status,
checkSuiteConclusion: checkSuite.conclusion,
runDatabaseId: run2.databaseId,
Expand All @@ -28238,29 +28237,34 @@ async function getCheckRunSummaries(token, params) {
});
return summaries.toSorted((a, b) => join(a.workflowPath, a.jobName).localeCompare(join(b.workflowPath, b.jobName)));
}
async function fetchOtherRunStatus(token, params, waitList, skipList) {
async function fetchOtherRunStatus(token, trigger, waitList, skipList, shouldSkipSameWorkflow) {
if (waitList.length > 0 && skipList.length > 0) {
throw new Error("Do not specify both wait-list and skip-list");
}
let checkRunSummaries = await getCheckRunSummaries(token, params);
const summaries = await getCheckRunSummaries(token, trigger);
const others = summaries.filter((summary) => !(summary.isSameWorkflow && trigger.jobName === summary.jobName));
let filtered = others.filter((summary) => !(summary.isSameWorkflow && shouldSkipSameWorkflow));
if (waitList.length > 0) {
checkRunSummaries = checkRunSummaries.filter(
filtered = filtered.filter(
(summary) => waitList.some(
(target) => target.workflowFile === summary.workflowPath && (target.jobName ? target.jobName === summary.jobName : true)
)
);
}
if (skipList.length > 0) {
checkRunSummaries = checkRunSummaries.filter(
filtered = filtered.filter(
(summary) => !skipList.some(
(target) => target.workflowFile === summary.workflowPath && (target.jobName ? target.jobName === summary.jobName : true)
)
);
}
const completedRuns = checkRunSummaries.filter((summary) => summary.runStatus === "COMPLETED");
const progress = completedRuns.length === checkRunSummaries.length ? "done" : "in_progress";
const conclusion = completedRuns.every((summary) => summary.acceptable) ? "acceptable" : "bad";
return { progress, conclusion, summaries: checkRunSummaries };
if (filtered.length === 0) {
throw new Error("No targets found except wait-other-jobs itself");
}
const completed = filtered.filter((summary) => summary.runStatus === "COMPLETED");
const progress = completed.length === filtered.length ? "done" : "in_progress";
const conclusion = completed.every((summary) => summary.acceptable) ? "acceptable" : "bad";
return { progress, conclusion, summaries: filtered };
}

// src/wait.ts
Expand Down Expand Up @@ -28315,6 +28319,11 @@ async function run() {
repo: { repo, owner },
payload,
runId,
runNumber,
// Another file can set same workflow name. So you should filter workfrows from runId or the filename
workflow,
// On the otherhand, jobName should be unique in each workflow from YAML spec
job,
sha
} = import_github.context;
const pr = payload.pull_request;
Expand Down Expand Up @@ -28355,18 +28364,22 @@ async function run() {
(0, import_core2.getInput)("attempt-limits", { required: true, trimWhitespace: true }),
10
);
const waitList = List.parse(JSON.parse((0, import_core2.getInput)("wait-list", { required: true })));
const skipList = List.parse(JSON.parse((0, import_core2.getInput)("skip-list", { required: true })));
const waitList = FilterConditions.parse(JSON.parse((0, import_core2.getInput)("wait-list", { required: true })));
const skipList = FilterConditions.parse(JSON.parse((0, import_core2.getInput)("skip-list", { required: true })));
if (waitList.length > 0 && skipList.length > 0) {
(0, import_core2.error)("Do not specify both wait-list and skip-list");
(0, import_core2.setFailed)("Specified both list");
}
const isEarlyExit = (0, import_core2.getBooleanInput)("early-exit", { required: true, trimWhitespace: true });
const shouldSkipSameWorkflow = (0, import_core2.getBooleanInput)("skip-same-workflow", { required: true, trimWhitespace: true });
const isDryRun = (0, import_core2.getBooleanInput)("dry-run", { required: true, trimWhitespace: true });
(0, import_core2.info)(JSON.stringify(
{
triggeredCommitSha: commitSha,
runId,
runNumber,
workflow,
job,
repositoryInfo,
waitSecondsBeforeFirstPolling,
minIntervalSeconds,
Expand All @@ -28375,7 +28388,8 @@ async function run() {
isEarlyExit,
isDryRun,
waitList,
skipList
skipList,
shouldSkipSameWorkflow
// Of course, do NOT include tokens here.
},
null,
Expand Down Expand Up @@ -28407,9 +28421,10 @@ async function run() {
(0, import_core2.startGroup)(`Polling ${attempts}: ${(/* @__PURE__ */ new Date()).toISOString()}`);
const report = await fetchOtherRunStatus(
githubToken,
{ ...repositoryInfo, ref: commitSha, triggerRunId: runId },
{ ...repositoryInfo, ref: commitSha, runId, jobName: job },
waitList,
skipList
skipList,
shouldSkipSameWorkflow
);
for (const summary of report.summaries) {
const {
Expand Down

0 comments on commit 9d1579c

Please sign in to comment.