Skip to content

Commit

Permalink
Remove retry logic
Browse files Browse the repository at this point in the history
The retries make debugging strictly more complex, and it's not clear that retrying provides actual value. If we need retries in the future, we should push them down into the per-API level.
  • Loading branch information
sethvargo committed Feb 2, 2024
1 parent 5a50e58 commit 2a88760
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 42 deletions.
10 changes: 7 additions & 3 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,24 +132,28 @@ inputs:
default: ''
required: false

# retries
# retries - TODO - remove in v3.0
retries:
description: |-
Number of times to retry a failed authentication attempt. This is useful
for automated pipelines that may execute before IAM permissions are fully
propogated.
default: '3'
deprecationMessage: |-
This field is no longer used and will be removed in a future release.
required: false
backoff:
description: |-
Delay time before trying another authentication attempt. This is
implemented using a fibonacci backoff method (e.g. 1-1-2-3-5). The default
value is 250 milliseconds.
default: '250'
deprecationMessage: |-
This field is no longer used and will be removed in a future release.
required: false
backoff_limit:
description: |-
Limits the retry backoff to the specified value.
deprecationMessage: |-
This field is no longer used and will be removed in a future release.
required: false

# id token params
Expand Down
6 changes: 3 additions & 3 deletions dist/main/index.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions dist/post/index.js

Large diffs are not rendered by default.

39 changes: 11 additions & 28 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
parseMultilineCSV,
parseDuration,
pinnedToHeadWarning,
withRetries,
} from '@google-github-actions/actions-utils';

import {
Expand Down Expand Up @@ -59,41 +58,23 @@ const oidcWarning =
`GitHub Actions workflow permissions are incorrect, or this job is being ` +
`run from a fork. For more information, please see https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token`;

/**
* Executes the main action.
*/
async function run(): Promise<void> {
export async function run() {
const logger = new Logger();

// Warn if pinned to HEAD
if (isPinnedToHead()) {
logger.warning(pinnedToHeadWarning('v2'));
}

const retries = Number(getInput('retries'));

// set to undefined when not provided [avoids Number('') -> 0]
const backoff = Number(getInput('backoff')) || undefined;
const backoffLimit = Number(getInput('backoff_limit')) || undefined;

try {
const mainWithRetries = withRetries(async () => main(logger), {
retries: retries,
backoff: backoff,
backoffLimit: backoffLimit,
});

await mainWithRetries();
main(logger);
} catch (err) {
const msg = errorMessage(err);
setFailed(`google-github-actions/auth failed with: ${msg}`);
}
}

/**
* Main wraps the main action logic into a function to be used as a parameter to the withRetries function.
*/
async function main(logger: Logger) {
export async function main(logger: Logger) {
// Warn if pinned to HEAD
if (isPinnedToHead()) {
logger.warning(pinnedToHeadWarning('v2'));
}

// Load configuration.
const projectID = computeProjectID(
getInput(`project_id`),
Expand Down Expand Up @@ -348,4 +329,6 @@ async function main(logger: Logger) {
}
}

run();
if (require.main === module) {
run();
}
9 changes: 4 additions & 5 deletions src/post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ import { errorMessage, forceRemove } from '@google-github-actions/actions-utils'

import { Logger } from './logger';

/**
* Executes the post action, documented inline.
*/
export async function run() {
const logger = new Logger();

Expand All @@ -32,7 +29,7 @@ export async function run() {
}
}

async function main(logger: Logger) {
export async function main(logger: Logger) {
const createCredentials = getBooleanInput('create_credentials_file');
if (!createCredentials) {
logger.info(`Skipping credential cleanup - "create_credentials_file" is false.`);
Expand Down Expand Up @@ -60,4 +57,6 @@ async function main(logger: Logger) {
logger.info(`Removed exported credentials at "${credentialsPath}".`);
}

run();
if (require.main === module) {
run();
}

0 comments on commit 2a88760

Please sign in to comment.