actions / toolkit

The GitHub ToolKit for developing GitHub Actions.
https://github.com/features/actions
MIT License
5.02k stars 1.45k forks source link

Expose `internalArtifactTwirpClient` `options` (especially `maxAttempts`) to `uploadArtifact` and friends #1864

Open jsoref opened 2 weeks ago

jsoref commented 2 weeks ago

Describe the enhancement

The current exported uploadArtifact api: https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/upload/upload-artifact.ts#L24-L28

Takes a parameter https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/shared/interfaces.ts#L20 That only exposes the two fields visible once an artifact is created: https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/shared/interfaces.ts#L36 https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/shared/interfaces.ts#L47

And it passes no arguments to internalArtifactTwirpClient https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/upload/upload-artifact.ts#L47

But, internalArtifactTwirpClient takes maxAttempts, retryIntervalMs, and retryMultiplier: https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/shared/artifact-twirp-client.ts#L184-L188

Code Snippet

export interface UploadArtifactSettings {
   maxAttempts?: number,
   retryIntervalMs?: number,
   retryMultiplier?: number, 
}

export interface UploadArtifactOptions { 
retentionDays?: number, 
compressionLevel?: number,
uploadSettings?: UploadArtifactSettings,
}
uploadArtifact("my-name", ["my-file"], ".", { uploadSettings: { maxAttempts: 2 } });

I think there's some value in distinguishing between values which are visible after an artifact is uploaded with the values for how the artifact is uploaded, hence sticking them in a sub object. I lean here towards extending the current options argument instead of adding a second options argument to the uploadArtifact call itself. But any approach would be fine.

Additional information There are people who trip on outages from this api, as seen in https://github.com/actions/upload-artifact/issues/569.

I'd like to be able to configure my actions/workflows to give up sooner (or try longer, but, in my case definitely give up sooner) when uploading an artifact fails.

In my case, two failed artifact uploads added 6 minutes to a 4 minute run. If I had CI for a private repository doing that, I'd expect those 6 minutes to be billed and that could add up over time.

While I'm mostly interested in exposing maxAttempts to uploadArtifact (and eventually getting it added to actions/upload-artifact), I think exposing retryIntervalMs and retryMultiplier isn't unreasonable. Similarly, I'd want to be able to use these options for downloadArtifact and listArtifacts as well...

https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/client.ts#L40-L45 https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/client.ts#L57-L59 https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/download/download-artifact.ts#L152-L158 https://github.com/actions/toolkit/blob/7f5921cdddc31081d4754a42711d71e7890b0d06/packages/artifact/src/internal/find/list-artifacts.ts#L115-L118


If exposing them directly through the API is too much, I'd be happy to have them exposed via environment variables as that would save me the effort of getting the middle consumers (actions/upload-artifact,...) to update and expose the knobs.