Skip to content

hotfix: remove gh artifact retrieval limit#636

Merged
grantfirl merged 2 commits into
NCAR:mainfrom
scrasmussen:hotfix/download-artifacts-limit
Nov 7, 2025
Merged

hotfix: remove gh artifact retrieval limit#636
grantfirl merged 2 commits into
NCAR:mainfrom
scrasmussen:hotfix/download-artifacts-limit

Conversation

@scrasmussen
Copy link
Copy Markdown
Member

@scrasmussen scrasmussen commented Nov 7, 2025

SOURCE: Soren Rasmussen, NCAR

DESCRIPTION OF CHANGES:

  • CI was failing since the gh command only finds the past 30 artifact ids by default. Adding the --paginate flag lists all artifacts that exist. Note, this doesn't list old ones, just ones that can be downloaded.

ISSUE: CI was failing since the gh command only finds the past 30 artifact ids by default. After enough PR CIs were run and created artifacts, the main branch artifacts were pushed off the returned list.

ASSOCIATED PRs:

TESTS CONDUCTED: List tests done as appropriate. Delete if not used.

@grantfirl
Copy link
Copy Markdown
Collaborator

Thanks @scrasmussen. I didn't understand exactly why it was failing, although I had an inkling that it had something to do with wiping artifacts after some threshold. To get around this, I was thinking that we can just have a workflow-dispatchable CI script to generate a fresh artifact from main. The current script could work in theory, but it errors out due to not being able to download existing artifacts before uploading newly-created ones.

I'm also wondering if we should change the triggers on the RT CIs to either only be workflow dispatchable or dispatchable via label. Having them automatically run every time there is a push to a PR is probably overkill.

@scrasmussen scrasmussen marked this pull request as draft November 7, 2025 16:54
@scrasmussen scrasmussen force-pushed the hotfix/download-artifacts-limit branch from d55acc2 to fb66281 Compare November 7, 2025 17:00
@scrasmussen
Copy link
Copy Markdown
Member Author

@grantfirl

to get around this, I was thinking that we can just have a workflow-dispatchable CI script to generate a fresh artifact from main.

These are good points, sorry this new testing feature didn't initially quite work as expected! I had considered adding a workflow-dispatchable CI script, what if the artifacts had expired from being too old, there would be no way to create new ones... I'll add a new script to this PR to do that.

but it errors out due to not being able to download existing artifacts before uploading newly-created ones.

I'm also wondering if we should change the triggers on the RT CIs to either only be workflow dispatchable or dispatchable via label. Having them automatically run every time there is a push to a PR is probably overkill.

I think there are pros and cons to doing it each way. Should new main artifacts only be pushed after they've been compared with the previous ones? Is it safer or should we consider any commit hitting the main branch to be truth by default, it would have gone through a vetting process already.

I prefer the overkill method, but am open to this discussion! I've added these points to our Tuesday SCM meeting notes so we can talk about them

…ml but without the download and comparing artifacts
@scrasmussen scrasmussen marked this pull request as ready for review November 7, 2025 17:26
@scrasmussen
Copy link
Copy Markdown
Member Author

scrasmussen commented Nov 7, 2025

This PR is ready for review, I can split off the artifact-creation dispatch file into a new PR if needed (it isn't really a hotfix so I'm overloading this PR, apologies)

@grantfirl
Copy link
Copy Markdown
Collaborator

This PR is ready for review, I can split off the artifact-creation dispatch file into a new PR if needed (it isn't really a hotfix so I'm overloading this PR, apologies)

No need, this is great. We should merge once CI finishes.

@grantfirl grantfirl merged commit 1a985b8 into NCAR:main Nov 7, 2025
12 checks passed
@scrasmussen scrasmussen deleted the hotfix/download-artifacts-limit branch November 7, 2025 19:31
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.

2 participants