Skip to content

Commit b5399f6

Browse files
Copilotnpalm
andcommitted
refactor: simplify naming and reduce complexity
Simplified the retry message logic based on reviewer feedback: - Renamed `validMessagesForRetry` to `queuedMessages` (clearer intent) - Renamed `invalidMessageIds` to `rejectedMessageIds` (more accurate) - Removed `invalidMessages` array duplication - now using only Set - Removed `publishRetryMessagesForValid` helper function (inline instead) - Convert Set to array only at return statement Result: clearer naming, less complexity, same functionality. Net reduction of 6 lines while maintaining all tests passing. Co-authored-by: npalm <11609620+npalm@users.noreply.github.com>
1 parent 11c1578 commit b5399f6

File tree

1 file changed

+19
-25
lines changed
  • lambdas/functions/control-plane/src/scale-runners

1 file changed

+19
-25
lines changed

lambdas/functions/control-plane/src/scale-runners/scale-up.ts

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,6 @@ function generateRunnerServiceConfig(githubRunnerConfig: CreateGitHubRunnerConfi
9090
return config;
9191
}
9292

93-
async function publishRetryMessagesForValid(
94-
messages: ActionRequestMessageSQS[],
95-
invalidMessageIds: Set<string>,
96-
): Promise<void> {
97-
for (const message of messages) {
98-
if (!invalidMessageIds.has(message.messageId)) {
99-
await publishRetryMessage(message as ActionRequestMessageRetry);
100-
}
101-
}
102-
}
103-
10493
async function getGithubRunnerRegistrationToken(githubRunnerConfig: CreateGitHubRunnerConfig, ghClient: Octokit) {
10594
const registrationToken =
10695
githubRunnerConfig.runnerType === 'Org'
@@ -286,8 +275,7 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
286275
};
287276

288277
const validMessages = new Map<string, MessagesWithClient>();
289-
const invalidMessages: string[] = [];
290-
const invalidMessageIds = new Set<string>();
278+
const rejectedMessageIds = new Set<string>();
291279
for (const payload of payloads) {
292280
const { eventType, messageId, repositoryName, repositoryOwner } = payload;
293281
if (ephemeralEnabled && eventType !== 'workflow_job') {
@@ -296,7 +284,7 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
296284
{ eventType, messageId },
297285
);
298286

299-
invalidMessages.push(messageId);
287+
rejectedMessageIds.add(messageId);
300288

301289
continue;
302290
}
@@ -351,7 +339,7 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
351339
for (const [group, { githubInstallationClient, messages }] of validMessages.entries()) {
352340
// Work out how much we want to scale up by.
353341
let scaleUp = 0;
354-
const validMessagesForRetry: ActionRequestMessageSQS[] = [];
342+
const queuedMessages: ActionRequestMessageSQS[] = [];
355343

356344
for (const message of messages) {
357345
const messageLogger = logger.createChild({
@@ -370,7 +358,7 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
370358
}
371359

372360
scaleUp++;
373-
validMessagesForRetry.push(message);
361+
queuedMessages.push(message);
374362
}
375363

376364
if (scaleUp === 0) {
@@ -407,14 +395,17 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
407395
// This removes `missingInstanceCount` items from the start of the array
408396
// so that, if we retry more messages later, we pick fresh ones.
409397
const removedMessages = messages.splice(0, missingInstanceCount);
410-
invalidMessages.push(...removedMessages.map(({ messageId }) => messageId));
411-
removedMessages.forEach(({ messageId }) => invalidMessageIds.add(messageId));
398+
removedMessages.forEach(({ messageId }) => rejectedMessageIds.add(messageId));
412399
}
413400

414401
// No runners will be created, so skip calling the EC2 API.
415402
if (newRunners <= 0) {
416-
// Publish retry messages for all remaining messages that are not marked as invalid
417-
await publishRetryMessagesForValid(validMessagesForRetry, invalidMessageIds);
403+
// Publish retry messages for messages that are not rejected
404+
for (const message of queuedMessages) {
405+
if (!rejectedMessageIds.has(message.messageId)) {
406+
await publishRetryMessage(message as ActionRequestMessageRetry);
407+
}
408+
}
418409
continue;
419410
}
420411
}
@@ -467,15 +458,18 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
467458
});
468459

469460
const failedMessages = messages.slice(0, failedInstanceCount);
470-
invalidMessages.push(...failedMessages.map(({ messageId }) => messageId));
471-
failedMessages.forEach(({ messageId }) => invalidMessageIds.add(messageId));
461+
failedMessages.forEach(({ messageId }) => rejectedMessageIds.add(messageId));
472462
}
473463

474-
// Publish retry messages for all messages that are not marked as invalid
475-
await publishRetryMessagesForValid(validMessagesForRetry, invalidMessageIds);
464+
// Publish retry messages for messages that are not rejected
465+
for (const message of queuedMessages) {
466+
if (!rejectedMessageIds.has(message.messageId)) {
467+
await publishRetryMessage(message as ActionRequestMessageRetry);
468+
}
469+
}
476470
}
477471

478-
return invalidMessages;
472+
return Array.from(rejectedMessageIds);
479473
}
480474

481475
export function getGitHubEnterpriseApiUrl() {

0 commit comments

Comments
 (0)