From 188abfc20bfe377096ef142c0f57d4d9cfdff38c Mon Sep 17 00:00:00 2001 From: Bethany Date: Wed, 9 Aug 2023 17:42:14 -0700 Subject: [PATCH] implement feedback --- packages/artifact/__tests__/retention.test.ts | 59 +++++++++++++++++++ packages/artifact/src/internal/client.ts | 18 +++++- .../artifact/src/internal/shared/config.ts | 7 +++ .../artifact/src/internal/upload/retention.ts | 34 +++++++++++ .../src/internal/upload/upload-artifact.ts | 18 ++---- 5 files changed, 123 insertions(+), 13 deletions(-) create mode 100644 packages/artifact/__tests__/retention.test.ts create mode 100644 packages/artifact/src/internal/upload/retention.ts diff --git a/packages/artifact/__tests__/retention.test.ts b/packages/artifact/__tests__/retention.test.ts new file mode 100644 index 00000000..898c52a0 --- /dev/null +++ b/packages/artifact/__tests__/retention.test.ts @@ -0,0 +1,59 @@ +import {Timestamp} from '../src/generated' +import * as retention from '../src/internal/upload/retention' + +describe('retention', () => { + beforeEach(() => { + delete process.env['GITHUB_RETENTION_DAYS'] + }) + it('should return the inputted retention days if it is less than the max retention days', () => { + // setup + const mockDate = new Date('2020-01-01') + jest.useFakeTimers().setSystemTime(mockDate) + process.env['GITHUB_RETENTION_DAYS'] = '90' + + const exp = retention.getExpiration(30) + + expect(exp).toBeDefined() + const expDate = Timestamp.toDate(exp!) // we check whether exp is defined above + const expected = new Date() + expected.setDate(expected.getDate() + 30) + + expect(expDate).toEqual(expected) + }) + + it('should return the max retention days if the inputted retention days is greater than the max retention days', () => { + // setup + const mockDate = new Date('2020-01-01') + jest.useFakeTimers().setSystemTime(mockDate) + process.env['GITHUB_RETENTION_DAYS'] = '90' + + const exp = retention.getExpiration(120) + + expect(exp).toBeDefined() + const expDate = Timestamp.toDate(exp!) // we check whether exp is defined above + const expected = new Date() + expected.setDate(expected.getDate() + 90) + + expect(expDate).toEqual(expected) + }) + + it('should return undefined if the inputted retention days is undefined', () => { + const exp = retention.getExpiration() + expect(exp).toBeUndefined() + }) + + it('should return the inputted retention days if there is no max retention days', () => { + // setup + const mockDate = new Date('2020-01-01') + jest.useFakeTimers().setSystemTime(mockDate) + + const exp = retention.getExpiration(30) + + expect(exp).toBeDefined() + const expDate = Timestamp.toDate(exp!) // we check whether exp is defined above + const expected = new Date() + expected.setDate(expected.getDate() + 30) + + expect(expDate).toEqual(expected) + }) +}) diff --git a/packages/artifact/src/internal/client.ts b/packages/artifact/src/internal/client.ts index 26546430..f827d882 100644 --- a/packages/artifact/src/internal/client.ts +++ b/packages/artifact/src/internal/client.ts @@ -2,6 +2,7 @@ import {UploadOptions} from './upload/upload-options' import {UploadResponse} from './upload/upload-response' import {uploadArtifact} from './upload/upload-artifact' import {warning} from '@actions/core' +import {isGhes} from './shared/config' export interface ArtifactClient { /** @@ -40,10 +41,25 @@ export class Client implements ArtifactClient { rootDirectory: string, options?: UploadOptions | undefined ): Promise { + if (isGhes()) { + warning( + `@actions/artifact v2 and upload-artifact v4 are not currently supported on GHES.` + ) + return { + success: false + } + } + try { return uploadArtifact(name, files, rootDirectory, options) } catch (error) { - warning(`Failed to upload artifact: ${error}`) + warning( + `Artifact upload failed with error: ${error}. + +Errors can be temporary, so please try again and optionally run the action with debug enabled for more information. + +If the error persists, please check whether Actions is running normally at [https://githubstatus.com](https://www.githubstatus.com).` + ) return { success: false } diff --git a/packages/artifact/src/internal/shared/config.ts b/packages/artifact/src/internal/shared/config.ts index 23949f20..d1946e96 100644 --- a/packages/artifact/src/internal/shared/config.ts +++ b/packages/artifact/src/internal/shared/config.ts @@ -13,3 +13,10 @@ export function getResultsServiceUrl(): string { } return resultsUrl } + +export function isGhes(): boolean { + const ghUrl = new URL( + process.env['GITHUB_SERVER_URL'] || 'https://github.com' + ) + return ghUrl.hostname.toUpperCase() !== 'GITHUB.COM' +} diff --git a/packages/artifact/src/internal/upload/retention.ts b/packages/artifact/src/internal/upload/retention.ts new file mode 100644 index 00000000..aa16ba1b --- /dev/null +++ b/packages/artifact/src/internal/upload/retention.ts @@ -0,0 +1,34 @@ +import {Timestamp} from '../../generated' +import * as core from '@actions/core' + +export function getExpiration(retentionDays?: number): Timestamp | undefined { + if (!retentionDays) { + return undefined + } + + const maxRetentionDays = getRetentionDays() + if (maxRetentionDays && maxRetentionDays < retentionDays) { + core.warning( + `Retention days cannot be greater than the maximum allowed retention set within the repository. Using ${maxRetentionDays} instead.` + ) + retentionDays = maxRetentionDays + } + + const expirationDate = new Date() + expirationDate.setDate(expirationDate.getDate() + retentionDays) + + return Timestamp.fromDate(expirationDate) +} + +function getRetentionDays(): number | undefined { + const retentionDays = process.env['GITHUB_RETENTION_DAYS'] + if (!retentionDays) { + return undefined + } + const days = parseInt(retentionDays) + if (isNaN(days)) { + return undefined + } + + return days +} diff --git a/packages/artifact/src/internal/upload/upload-artifact.ts b/packages/artifact/src/internal/upload/upload-artifact.ts index 087493a3..84a2e90e 100644 --- a/packages/artifact/src/internal/upload/upload-artifact.ts +++ b/packages/artifact/src/internal/upload/upload-artifact.ts @@ -1,6 +1,7 @@ import * as core from '@actions/core' import {UploadOptions} from './upload-options' import {UploadResponse} from './upload-response' +import {getExpiration} from './util' import {validateArtifactName} from './path-and-artifact-name-validation' import {createArtifactTwirpClient} from '../shared/artifact-twirp-client' import { @@ -9,7 +10,7 @@ import { validateRootDirectory } from './upload-zip-specification' import {getBackendIdsFromToken} from '../shared/util' -import {CreateArtifactRequest, Timestamp} from 'src/generated' +import {CreateArtifactRequest} from 'src/generated' export async function uploadArtifact( name: string, @@ -39,6 +40,10 @@ export async function uploadArtifact( success: false } } + core.debug(`Workflow Run Backend ID: ${backendIds.workflowRunBackendId}`) + core.debug( + `Workflow Job Run Backend ID: ${backendIds.workflowJobRunBackendId}` + ) // create the artifact client const artifactClient = createArtifactTwirpClient('upload') @@ -91,14 +96,3 @@ export async function uploadArtifact( return uploadResponse } - -function getExpiration(retentionDays?: number): Timestamp | undefined { - if (!retentionDays) { - return undefined - } - - const expirationDate = new Date() - expirationDate.setDate(expirationDate.getDate() + retentionDays) - - return Timestamp.fromDate(expirationDate) -}