Skip to content

Commit 5019667

Browse files
authored
Fix maintainer-approval runner and clean up OWNERS (#4914)
## Why The maintainer-approval workflow from #4912 used `ubuntu-latest` (GitHub public runners), which can't make API calls against the `databricks` org due to the IP allow-list. Also, `@simonfaltum` was listed on the `/cmd/bundle/` OWNERS rule but isn't on the bundle team. ## Changes 1. Switch `maintainer-approval.yml` from `ubuntu-latest` to the `databricks-deco-testing-runner-group` custom runner, matching all other workflows in the repo. 2. Remove `@simonfaltum` from `/cmd/bundle/` in `.github/OWNERS` since he's not on the bundle team. ## Test plan - [x] `suggest-reviewers / suggest-reviewers` fired and posted a reviewer suggestion comment with round-robin fallback - [ ] After merge, verify `Maintainer approval / check` runs on the custom runner (requires a new PR since `pull_request_target` reads workflows from the base branch)
1 parent b6c6a56 commit 5019667

File tree

4 files changed

+24
-22
lines changed

4 files changed

+24
-22
lines changed

.github/OWNERS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
* @andrewnester @anton-107 @denik @pietern @shreyas-goenka @simonfaltum
2-
/cmd/bundle/ @andrewnester @anton-107 @denik @pietern @shreyas-goenka @simonfaltum @lennartkats-db
2+
/cmd/bundle/ @andrewnester @anton-107 @denik @pietern @shreyas-goenka @lennartkats-db
33
/libs/template/ @andrewnester @anton-107 @denik @pietern @shreyas-goenka @simonfaltum @lennartkats-db
44
/acceptance/pipelines/ @jefferycheng1 @kanterov @lennartkats-db
55
/cmd/pipelines/ @jefferycheng1 @kanterov @lennartkats-db

.github/scripts/owners.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,12 @@ function findOwners(filepath, rules) {
5757
}
5858

5959
/**
60-
* Get core team from the * catch-all rule.
60+
* Get maintainers from the * catch-all rule.
6161
* @returns {string[]} logins
6262
*/
63-
function getCoreTeam(rules) {
63+
function getMaintainers(rules) {
6464
const catchAll = rules.find((r) => r.pattern === "*");
6565
return catchAll ? catchAll.owners : [];
6666
}
6767

68-
module.exports = { parseOwnersFile, ownersMatch, findOwners, getCoreTeam };
68+
module.exports = { parseOwnersFile, ownersMatch, findOwners, getMaintainers };

.github/workflows/maintainer-approval.js

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,19 @@ const path = require("path");
22
const {
33
parseOwnersFile,
44
findOwners,
5-
getCoreTeam,
5+
getMaintainers,
66
} = require("../scripts/owners");
77

88
// Check if the PR author is exempted.
9-
// If ALL changed files are owned by non-core-team owners that include the
10-
// author, the PR can merge with any approval (not necessarily core team).
11-
function isExempted(authorLogin, files, rules, coreTeam) {
9+
// If ALL changed files are owned by non-maintainer owners that include the
10+
// author, the PR can merge with any approval (not necessarily a maintainer).
11+
function isExempted(authorLogin, files, rules, maintainers) {
1212
if (files.length === 0) return false;
13-
const coreSet = new Set(coreTeam);
13+
const maintainerSet = new Set(maintainers);
1414
for (const { filename } of files) {
1515
const owners = findOwners(filename, rules);
16-
const nonCoreOwners = owners.filter((o) => !coreSet.has(o));
17-
if (nonCoreOwners.length === 0 || !nonCoreOwners.includes(authorLogin)) {
16+
const nonMaintainers = owners.filter((o) => !maintainerSet.has(o));
17+
if (nonMaintainers.length === 0 || !nonMaintainers.includes(authorLogin)) {
1818
return false;
1919
}
2020
}
@@ -28,11 +28,11 @@ module.exports = async ({ github, context, core }) => {
2828
"OWNERS"
2929
);
3030
const rules = parseOwnersFile(ownersPath);
31-
const coreTeam = getCoreTeam(rules);
31+
const maintainers = getMaintainers(rules);
3232

33-
if (coreTeam.length === 0) {
33+
if (maintainers.length === 0) {
3434
core.setFailed(
35-
"Could not determine core team from .github/OWNERS (no * rule found)."
35+
"Could not determine maintainers from .github/OWNERS (no * rule found)."
3636
);
3737
return;
3838
}
@@ -43,17 +43,17 @@ module.exports = async ({ github, context, core }) => {
4343
pull_number: context.issue.number,
4444
});
4545

46-
const coreTeamApproved = reviews.some(
46+
const maintainerApproved = reviews.some(
4747
({ state, user }) =>
48-
state === "APPROVED" && user && coreTeam.includes(user.login)
48+
state === "APPROVED" && user && maintainers.includes(user.login)
4949
);
5050

51-
if (coreTeamApproved) {
51+
if (maintainerApproved) {
5252
const approver = reviews.find(
5353
({ state, user }) =>
54-
state === "APPROVED" && user && coreTeam.includes(user.login)
54+
state === "APPROVED" && user && maintainers.includes(user.login)
5555
);
56-
core.info(`Core team approval from @${approver.user.login}`);
56+
core.info(`Maintainer approval from @${approver.user.login}`);
5757
return;
5858
}
5959

@@ -67,7 +67,7 @@ module.exports = async ({ github, context, core }) => {
6767
pull_number: context.issue.number,
6868
});
6969

70-
if (authorLogin && isExempted(authorLogin, files, rules, coreTeam)) {
70+
if (authorLogin && isExempted(authorLogin, files, rules, maintainers)) {
7171
const hasAnyApproval = reviews.some(({ state }) => state === "APPROVED");
7272
if (!hasAnyApproval) {
7373
core.setFailed(
@@ -78,6 +78,6 @@ module.exports = async ({ github, context, core }) => {
7878
}
7979

8080
core.setFailed(
81-
`Requires approval from a core team member: ${coreTeam.join(", ")}.`
81+
`Requires approval from a maintainer: ${maintainers.join(", ")}.`
8282
);
8383
};

.github/workflows/maintainer-approval.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ defaults:
1111

1212
jobs:
1313
check:
14-
runs-on: ubuntu-latest
14+
runs-on:
15+
group: databricks-deco-testing-runner-group
16+
labels: ubuntu-latest-deco
1517
timeout-minutes: 5
1618
permissions:
1719
pull-requests: read

0 commit comments

Comments
 (0)