Skip to content

fix: bumping max list artifact to 2k#2161

Closed
functionstackx wants to merge 2 commits into
actions:mainfrom
functionstackx:max-list-artifact-2k
Closed

fix: bumping max list artifact to 2k#2161
functionstackx wants to merge 2 commits into
actions:mainfrom
functionstackx:max-list-artifact-2k

Conversation

@functionstackx
Copy link
Copy Markdown
Contributor

running into use cases where each workflow has more than 1k artifacts that i need to download

@functionstackx functionstackx requested a review from a team as a code owner October 17, 2025 03:14
Copilot AI review requested due to automatic review settings October 17, 2025 03:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR increases the maximum artifact count limit from 1,000 to 2,000 to accommodate workflows with larger numbers of artifacts. The change updates both the limit constant and the corresponding warning message to reflect the new threshold.

Key Changes:

  • Doubled the maximum artifact count limit from 1,000 to 2,000
  • Updated the warning message to use the dynamic maximumArtifactCount variable instead of a hardcoded value

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread packages/artifact/src/internal/find/list-artifacts.ts Outdated
Copy link
Copy Markdown
Contributor

@austenstone austenstone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. I know this has been tested.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
// Limiting to 1000 for perf reasons
const maximumArtifactCount = 1000
// Limiting to 2000 for perf reasons
const maximumArtifactCount = 2000
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@functionstackx - Instead of making this change, I recommend that you add a helper function to actions/toolkit/packages/artifact/src/internal/shared/config.ts which will read an environment variable let's call it: ACTIONS_ARTIFACT_MAX_ARTIFACT_LIST_COUNT

The default being 1000 if this env var is not set.

This way we make this configurable and avoid causing others to hit their primary rate limits with this change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change here is not sufficient alone of course, we'll need to cut a new release for this package, pull it in: https://github.com/actions/download-artifact and also cut a new minor release for that action.

Copy link
Copy Markdown
Member

@Link- Link- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @functionstackx for your contribution! I left a recommendation for a better approach. Feel free to tag me when you have the changes made.

Also, please make sure to add tests in either:

  • actions/toolkit/packages/artifact/__tests__/download-artifact.test.ts
  • actions/toolkit/packages/artifact/__tests__/list-artifacts.test.ts

to make sure we cover the changes

@austenstone
Copy link
Copy Markdown
Contributor

Continuing the work here
#2165

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants