Skip to content

all-repos: update actions/upload-artifact to v4 #3381

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 2 commits into from

Conversation

joshuarli
Copy link
Member

@sentrivana
Copy link
Contributor

Looks like there's a 409 being thrown with the new upload-artifact -- might be we are affected by actions/upload-artifact#478. I see folks in the thread solving this by giving the artifact a unique name. We need to look into this soon since v3 is getting deprecated.

@joshuarli
Copy link
Member Author

joshuarli commented Jul 31, 2024

edit: will need to take a closer look at how github release assets are done, have a feeling things might break if artifact names are renamed

Looks like there's a 409 being thrown with the new upload-artifact -- might be we are affected by actions/upload-artifact#478. I see folks in the thread solving this by giving the artifact a unique name. We need to look into this soon since v3 is getting deprecated.

Yup, a unique name is what's needed. I propose ${{ github.sha }}-dist and ${{ github.sha }}-gh-pages, what do you think?

Also, it would be better to save on some unnecessary(?) work if upload-artifact were skipped if it's a PR branch. Let me know if you want me to do that.

Copy link

codecov bot commented Jul 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.71%. Comparing base (2c1e31c) to head (9c8f5b3).
Report is 47 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3381   +/-   ##
=======================================
  Coverage   79.71%   79.71%           
=======================================
  Files         132      132           
  Lines       14264    14264           
  Branches     3003     3003           
=======================================
  Hits        11370    11370           
- Misses       2071     2072    +1     
+ Partials      823      822    -1     

see 2 files with indirect coverage changes

@sentrivana
Copy link
Contributor

@joshuarli Both suggestions sound good to me. I guess instead of renaming we could also do overwrite: true?

@joshuarli
Copy link
Member Author

joshuarli commented Aug 2, 2024

@joshuarli Both suggestions sound good to me. I guess instead of renaming we could also do overwrite: true?

i don't think we generally want that, it deletes anything under the previous artifact name

see getsentry/craft#552

@joshuarli
Copy link
Member Author

Superseded by #3545.

@joshuarli joshuarli closed this Sep 17, 2024
@joshuarli joshuarli deleted the all-repos_autofix_all-repos-sed branch September 17, 2024 19:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants