-
-
Notifications
You must be signed in to change notification settings - Fork 26
Cache activity before opening retirement PRs #120
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| .tmp | ||
| .cache |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,7 +75,10 @@ To test: | |
| PROD=1 scripts/nomination.sh members infinisil-test-org/nixpkgs-committers 33 30 <<< "added members/infinisil" | ||
| ``` | ||
|
|
||
| ## Testing `retire.sh` | ||
| ## Testing `retire.js` | ||
|
|
||
| In CI, `retire.js` runs via [`actions/github-script`](https://github.com/actions/github-script). | ||
| Locally it can be run with `scripts/retire-local.js`, which needs Node.js (`pkgs.nodejs`) and uses the `gh` CLI for API access instead. | ||
|
Comment on lines
+80
to
+81
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add a They'd still need to configure |
||
|
|
||
| This script has external effects and as such needs a bit more care when testing. | ||
|
|
||
|
|
@@ -102,51 +105,69 @@ git push -f -u origin HEAD | |
|
|
||
| ### Test sequence | ||
|
|
||
| The following sequence tests all code paths: | ||
| The following sequence tests all code paths. | ||
|
|
||
| The `CONFIRM_CUTOFF` argument is passed as `now` so that a single run is enough to open a retirement PR. | ||
| In CI it is set to `7 days ago` instead, which together with the `CACHE_FILE` activity cache (persisted between runs as a workflow artifact) requires a week of daily runs to agree before a PR is opened. | ||
|
|
||
| 1. Run the script with the `active` repo argument to simulate CI running without inactive users: | ||
| ```bash | ||
| scripts/retire.sh infinisil-test-org active nixpkgs-committers members-test 'yesterday 1 month ago' now | ||
| scripts/retire-local.js infinisil-test-org active nixpkgs-committers members-test 'yesterday 1 month ago' now now | ||
| ``` | ||
|
|
||
| Check that no PR would be opened. | ||
| 1. Run the previous command again with `CACHE_FILE` set: | ||
| ```bash | ||
| CACHE_FILE=.tmp-cache.json scripts/retire-local.js infinisil-test-org active nixpkgs-committers members-test 'yesterday 1 month ago' now now | ||
| CACHE_FILE=.tmp-cache.json scripts/retire-local.js infinisil-test-org active nixpkgs-committers members-test 'yesterday 1 month ago' now now | ||
| rm .tmp-cache.json | ||
| ``` | ||
|
|
||
| Check that the first run writes your activity to the cache file and that the second run skips the `/commits` query for your user because of it. | ||
| 1. Run the script with the `empty` repo argument to simulate CI running with inactive users: | ||
|
|
||
| ```bash | ||
| scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now | ||
| scripts/retire-local.js infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now now | ||
| ``` | ||
|
|
||
| Check that it would only create a PR for your own user and not the "new-committer-1" or "new-committer-2" user. | ||
| Also check that with a `CONFIRM_CUTOFF` in the past (as used in CI), no PR would be opened yet: | ||
|
|
||
| ```bash | ||
| scripts/retire-local.js infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now '7 days ago' | ||
| ``` | ||
|
|
||
| Check that it would only create a PR for your own user and not the "new-committer-1" or "new-committer-2" user before running it again with `PROD=1` to actually do it: | ||
| Then run it again with `PROD=1` to actually do it: | ||
|
|
||
| ```bash | ||
| PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now | ||
| PROD=1 scripts/retire-local.js infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now now | ||
| ``` | ||
|
|
||
| Check that it created the PR appropriately, including assigning the "retirement" label. | ||
| You can undo this step by closing the PR. | ||
| 1. Run it again to simulate CI running again later: | ||
| ```bash | ||
| PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now | ||
| PROD=1 scripts/retire-local.js infinisil-test-org empty nixpkgs-committers members-test 'yesterday 1 month ago' now now | ||
| ``` | ||
| Check that no other PR is opened. | ||
| 1. Run it again with `now` as the notice cutoff date to simulate the time interval passing: | ||
| ```bash | ||
| PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test now now | ||
| PROD=1 scripts/retire-local.js infinisil-test-org empty nixpkgs-committers members-test now now now | ||
| ``` | ||
| Check that it undrafted the previous PR and posted an appropriate comment. | ||
| 1. Run it again to simulate CI running again later: | ||
| ```bash | ||
| PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test now now | ||
| PROD=1 scripts/retire-local.js infinisil-test-org empty nixpkgs-committers members-test now now now | ||
| ``` | ||
| Check that no other PR is opened. | ||
| 1. Reset by marking the PR as a draft again, then run it again with the `active` repo argument to simulate activity during the time interval: | ||
| ```bash | ||
| PROD=1 scripts/retire.sh infinisil-test-org active nixpkgs-committers members-test now now | ||
| PROD=1 scripts/retire-local.js infinisil-test-org active nixpkgs-committers members-test now now now | ||
| ``` | ||
| Check that it gets undrafted with a comment listing the new activity. | ||
| 1. Close the PR, then run the script again with no activity and for an earlier close cutoff, simulating that the retirement was delayed: | ||
| ```bash | ||
| PROD=1 scripts/retire.sh infinisil-test-org empty nixpkgs-committers members-test now '1 day ago' | ||
| PROD=1 scripts/retire-local.js infinisil-test-org empty nixpkgs-committers members-test now '1 day ago' now | ||
| ``` | ||
|
|
||
| Check that no other PR is opened. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| // Restores the activity cache artifact uploaded by the previous successful | ||
| // run of the retire workflow, for use by scripts/retire.js. | ||
| // Runs in CI via actions/github-script (see .github/workflows/retire.yml). | ||
| // | ||
| // Any failure here only means starting without a cache, which is safe: | ||
| // a missing cache can delay retirements, but never cause one. | ||
|
|
||
| 'use strict'; | ||
|
|
||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
| const { execFileSync } = require('child_process'); | ||
|
|
||
| module.exports = async function restoreCache({ github, context, core, artifactName, dir }) { | ||
| const { owner, repo } = context.repo; | ||
| try { | ||
| const runs = ( | ||
| await github.request(`GET /repos/${owner}/${repo}/actions/workflows/retire.yml/runs`, { | ||
| status: 'success', | ||
| per_page: 1, | ||
| }) | ||
| ).data.workflow_runs; | ||
| if (runs.length === 0) { | ||
| core.info('No previous successful run, starting fresh'); | ||
| return; | ||
| } | ||
| const runId = runs[0].id; | ||
|
|
||
| const artifacts = ( | ||
| await github.request(`GET /repos/${owner}/${repo}/actions/runs/${runId}/artifacts`) | ||
| ).data.artifacts; | ||
| const artifact = artifacts.find((a) => a.name === artifactName && !a.expired); | ||
| if (!artifact) { | ||
| core.warning(`Run ${runId} has no ${artifactName} artifact, starting fresh`); | ||
| return; | ||
| } | ||
|
|
||
| // The artifact download endpoint always serves a zip archive | ||
| const zip = ( | ||
| await github.request(`GET /repos/${owner}/${repo}/actions/artifacts/${artifact.id}/zip`) | ||
| ).data; | ||
| fs.mkdirSync(dir, { recursive: true }); | ||
| const zipPath = path.join(dir, `${artifactName}.zip`); | ||
| fs.writeFileSync(zipPath, Buffer.from(zip)); | ||
| execFileSync('unzip', ['-o', zipPath, '-d', dir], { stdio: 'inherit' }); | ||
| fs.rmSync(zipPath); | ||
| core.info(`Restored activity cache from run ${runId}`); | ||
|
Comment on lines
+38
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uploading with (I don't have permission to unresolve #120 (comment), but see my reply there) |
||
| } catch (err) { | ||
| core.warning(`Could not restore activity cache, starting fresh: ${err.message}`); | ||
| } | ||
| }; | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,74 @@ | ||||||
| #!/usr/bin/env node | ||||||
| // Local runner for scripts/retire.js, providing an Octokit-compatible shim | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Let's avoid implying the shim is fully compatible, since it only covers the subset we use. |
||||||
| // backed by the `gh` CLI so no npm dependencies are needed. | ||||||
| // See scripts/README.md for the test sequence. | ||||||
|
|
||||||
| 'use strict'; | ||||||
|
|
||||||
| const { execFileSync } = require('child_process'); | ||||||
| const retire = require('./retire.js'); | ||||||
|
|
||||||
| function usage() { | ||||||
| console.error(`Usage: ${process.argv[1]} ORG ACTIVITY_REPO MEMBER_REPO DIR NOTICE_CUTOFF CLOSE_CUTOFF CONFIRM_CUTOFF`); | ||||||
| console.error(''); | ||||||
| console.error('Optionally set CACHE_FILE to a path for persisting activity observations between runs.'); | ||||||
| console.error('Set PROD=1 to actually perform side effects.'); | ||||||
| process.exit(1); | ||||||
| } | ||||||
|
|
||||||
| const [org, activityRepo, memberRepo, dir, noticeCutoffSpec, closeCutoffSpec, confirmCutoffSpec] = | ||||||
| process.argv.slice(2); | ||||||
| if (!confirmCutoffSpec) { | ||||||
| usage(); | ||||||
| } | ||||||
|
|
||||||
| function gh(args, input) { | ||||||
| return execFileSync('gh', args, { | ||||||
| input, | ||||||
| encoding: 'utf8', | ||||||
| maxBuffer: 64 * 1024 * 1024, | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| const github = { | ||||||
| async request(route, params = {}) { | ||||||
| const [method, pathTemplate] = route.split(' '); | ||||||
| let args; | ||||||
| let input; | ||||||
| if (method === 'GET') { | ||||||
| const query = new URLSearchParams(params).toString(); | ||||||
| args = ['api', '-X', 'GET', query ? `${pathTemplate}?${query}` : pathTemplate]; | ||||||
| } else { | ||||||
| args = ['api', '-X', method, pathTemplate, '--input', '-']; | ||||||
| input = JSON.stringify(params); | ||||||
| } | ||||||
| const out = gh(args, input); | ||||||
| return { data: out.trim() === '' ? null : JSON.parse(out) }; | ||||||
| }, | ||||||
| async graphql(query, variables = {}) { | ||||||
| const out = gh(['api', 'graphql', '--input', '-'], JSON.stringify({ query, variables })); | ||||||
| const parsed = JSON.parse(out); | ||||||
| if (parsed.errors) { | ||||||
| throw new Error(JSON.stringify(parsed.errors)); | ||||||
| } | ||||||
| return parsed.data; | ||||||
| }, | ||||||
| }; | ||||||
|
|
||||||
| retire({ | ||||||
| github, | ||||||
| options: { | ||||||
| org, | ||||||
| activityRepo, | ||||||
| memberRepo, | ||||||
| dir, | ||||||
| noticeCutoffSpec, | ||||||
| closeCutoffSpec, | ||||||
| confirmCutoffSpec, | ||||||
| cacheFile: process.env.CACHE_FILE, | ||||||
| prod: Boolean(process.env.PROD), | ||||||
| }, | ||||||
| }).catch((err) => { | ||||||
| console.error(err); | ||||||
| process.exit(1); | ||||||
| }); | ||||||
Uh oh!
There was an error while loading. Please reload this page.