Skip to content

Commit 6ade929

Browse files
🐛 individually add copilot teams (#77)
* This prevents overflow issues due to large amounts of copilot teams while also allowing the overall call to succeed when one team in the batch is bad. * This change is being made after new failures were found where 1 team with no members would cause the entire batch of additions to fail.
1 parent 4c42c9f commit 6ade929

4 files changed

Lines changed: 55 additions & 30 deletions

File tree

src/services/gitHub.ts

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Octokit } from "octokit";
22
import { createAppAuth } from "@octokit/auth-app";
33
import { Config } from "../config";
4-
import { AddMemberResponse, GitHubClient, GitHubId, GitHubTeamId, InstalledClient, Org, OrgInvite, OrgRoles, RemoveMemberResponse, Response } from "./gitHubTypes";
4+
import { AddMemberResponse, CopilotAddResponse, GitHubClient, GitHubId, GitHubTeamId, InstalledClient, Org, OrgInvite, OrgRoles, RemoveMemberResponse, Response } from "./gitHubTypes";
55
import { AppConfig } from "./appConfig";
66
import yaml from "js-yaml";
77
import { throttling } from "@octokit/plugin-throttling";
@@ -230,7 +230,7 @@ class InstalledGitHubClient implements InstalledClient {
230230
this.orgName = orgName;
231231
}
232232

233-
async AddTeamsToCopilotSubscription(teamNames: string[]): Response<string[]> {
233+
async AddTeamsToCopilotSubscription(teamNames: string[]): Response<CopilotAddResponse[]> {
234234
// Such logic should not generally go in a facade, though the convenience
235235
// and lack of actual problems makes this violation of pattern more "okay."
236236
if (teamNames.length < 1) {
@@ -241,33 +241,45 @@ class InstalledGitHubClient implements InstalledClient {
241241
}
242242
}
243243

244-
try {
245-
const response = await this.gitHubClient.request("POST /orgs/{org}/copilot/billing/selected_teams", {
246-
org: this.orgName,
247-
selected_teams: teamNames,
248-
headers: {
249-
'X-GitHub-Api-Version': '2022-11-28'
250-
}
251-
});
252-
253-
if (response.status < 200 || response.status > 299) {
254-
return {
255-
successful: false
244+
const responses: CopilotAddResponse[] = [];
245+
246+
for (const team of teamNames) {
247+
try {
248+
const response = await this.gitHubClient.request("POST /orgs/{org}/copilot/billing/selected_teams", {
249+
org: this.orgName,
250+
selected_teams: [team],
251+
headers: {
252+
'X-GitHub-Api-Version': '2022-11-28'
253+
}
254+
});
255+
256+
if (response.status < 200 || response.status > 299) {
257+
responses.push({
258+
successful: false,
259+
team: team,
260+
message: response.status.toString()
261+
});
256262
}
257-
}
258263

259-
return {
260-
successful: true,
261-
data: teamNames
264+
responses.push({
265+
successful: true,
266+
team: team
267+
});
262268
}
263-
}
264-
catch (e) {
265-
console.log(e);
266-
// TODO: actually catch exception and investigate...
267-
return {
268-
successful: false
269+
catch (e) {
270+
console.log(e);
271+
responses.push({
272+
successful: false,
273+
team: team,
274+
message: JSON.stringify(e)
275+
});
269276
}
270277
}
278+
279+
return {
280+
successful: true,
281+
data: responses
282+
};
271283
}
272284

273285
async ListPendingInvitesForTeam(teamName: string): Response<OrgInvite[]> {

src/services/gitHubCache.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { CacheClient } from "../app";
22
import { ILogger } from "../logging";
3-
import { AddMemberResponse, GitHubId, GitHubTeamId, InstalledClient, OrgInvite, OrgRoles, RemoveMemberResponse, Response } from "./gitHubTypes";
3+
import { AddMemberResponse, CopilotAddResponse, GitHubId, GitHubTeamId, InstalledClient, OrgInvite, OrgRoles, RemoveMemberResponse, Response } from "./gitHubTypes";
44
import { OrgConfig } from "./orgConfig";
55

66
export class GitHubClientCache implements InstalledClient {
@@ -13,7 +13,7 @@ export class GitHubClientCache implements InstalledClient {
1313
this.cacheClient = cacheClient;
1414
this.logger = logger;
1515
}
16-
AddTeamsToCopilotSubscription(teamNames: string[]): Response<string[]> {
16+
AddTeamsToCopilotSubscription(teamNames: string[]): Response<CopilotAddResponse[]> {
1717
return this.client.AddTeamsToCopilotSubscription(teamNames);
1818
}
1919

src/services/gitHubTypes.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,20 @@ export interface InstalledClient {
3838
GetPendingOrgInvites():Response<OrgInvite[]>
3939
CancelOrgInvite(invite:OrgInvite): Response
4040
ListPendingInvitesForTeam(teamName: GitHubTeamName):Response<OrgInvite[]>
41-
AddTeamsToCopilotSubscription(teamNames: GitHubTeamName[]):Response<string[]>
41+
AddTeamsToCopilotSubscription(teamNames: GitHubTeamName[]):Response<CopilotAddResponse[]>
42+
}
43+
44+
export type CopilotAddResponse = CopilotAddSucceeded | CopilotAddFailed
45+
46+
export type CopilotAddSucceeded = {
47+
successful: true,
48+
team: GitHubTeamName
49+
}
50+
51+
export type CopilotAddFailed = {
52+
successful: false,
53+
message: string,
54+
team: GitHubTeamName
4255
}
4356

4457
export type AddMemberResponse = Promise<AddMemberSucceeded | AddMemberFailed>

src/services/githubSync.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { Log, LogError } from "../logging";
44
import { AppConfig } from "./appConfig";
5-
import { FailedResponse, GitHubId, InstalledClient, OrgInvite } from "./gitHubTypes";
5+
import { CopilotAddResponse, FailedResponse, GitHubId, InstalledClient, OrgInvite } from "./gitHubTypes";
66
import { IGitHubInvitations } from "./githubInvitations";
77
import { SearchAllAsync } from "./ldapClient";
88
import { OrgConfig } from "./orgConfig";
@@ -164,7 +164,7 @@ type ReturnTypeOfSyncOrg = {
164164
syncedSecurityManagerTeams: string[];
165165
orgOwnersGroup: string;
166166
ignoredTeams: string[];
167-
copilotTeams: string[];
167+
copilotTeams: CopilotAddResponse[];
168168
}
169169

170170
type FailedSecSync = {
@@ -230,7 +230,7 @@ async function syncOrg(installedGitHubClient: InstalledClient, appConfig: AppCon
230230
syncedSecurityManagerTeams: [] as string[],
231231
orgOwnersGroup: "",
232232
ignoredTeams: [] as string[],
233-
copilotTeams: [] as string[]
233+
copilotTeams: [] as CopilotAddResponse[]
234234
}
235235

236236
// TODO: add this back once these APIs make sense

0 commit comments

Comments
 (0)