Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Refactor artifact cleanup #2649

Merged
merged 2 commits into from
Aug 1, 2023
Merged

Conversation

guineveresaenger
Copy link
Contributor

Fixes #2603

Co-authored by: Alex Qiu [email protected]

@@ -103,6 +103,7 @@ jobs:
with:
name: ${{ matrix.language }}-sdk.tar.gz
path: ${{ github.workspace}}/sdk/${{ matrix.language }}.tar.gz
retention-days: 30
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no equivalent of the previous skip-tags: true here. Doesn't that mean we'll lose release artifacts after 30 days?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it would! Sounds like I was too eager here, and we should not add this to the release or prerelease workflows, using the default instead.

Copy link
Member

Choose a reason for hiding this comment

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

I think I saw you mentioned somewhere that 30 is an override of the default value which is 90. Don't we have the same problem after 90 days currently?

Copy link
Contributor Author

@guineveresaenger guineveresaenger Jul 30, 2023

Choose a reason for hiding this comment

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

@mikhailshilkov: This is the current status quo as far as I'm aware. Here is a release run of 2 months ago with existing artifacts. Compare it to this release of 3 months ago - the artifacts are Expired.
If we want to keep them around for longer, we would have to come up with a new idea, but I'm unaware of any need for that.
TL;DR: The PR in its current state does not change the expiration duration of release (tagged) artifacts for the shorter.

@guineveresaenger guineveresaenger force-pushed the guin/cleanup-artifacts-cleanup branch from e5f56f2 to 6518cb8 Compare August 1, 2023 04:24
@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

guineveresaenger added a commit to pulumi/pulumi-azure-native that referenced this pull request Aug 1, 2023
This PR removes the artifact cleanup cron in favor of [setting retention
days on the artifact upload
itself](https://github.blog/changelog/2020-10-08-github-actions-ability-to-change-retention-days-for-artifacts-and-logs/).
Fixes #2558.
Similar to pulumi/pulumi-aws#2649
@guineveresaenger guineveresaenger merged commit 42b0a0a into master Aug 1, 2023
@guineveresaenger guineveresaenger deleted the guin/cleanup-artifacts-cleanup branch August 1, 2023 18:10
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.

Workflow failure: cleanup
3 participants