Conversation
Generated by 🚫 Danger |
Complement the existing `upload_to_s3` action with a `delete_from_s3` action that supports single-key deletion, bulk deletion by prefix, age-based filtering (`older_than_days`), and a `dry_run` preview mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
13d22e3 to
40c8f3d
Compare
That's a great goal!
Got a question: what about setting up a Lifecycle rule on those paths in the S3 bucket instead, to let S3 do the cleanup for us instead of having to set up periodic cleanup jobs ourselves? |
I'm generally not a fan of this for setups like ours because I'd want each repo to be able to decide how long it stores stuff for. If I want to be more aggressive and drop it down to 90 days, IMHO I shouldn't have to go touch another repo to do it. It'd work, it just seems like it's mixing concerns across repos IMHO? |
Yeah, I see your point. Counter argument though: what if you want to run the cleanup cron job more frequently? You'd still have to go touch
Depends on your perspective I guess. Personally I just think that this approach is not the right solution to reach for to solve the problem at hand. I actually think that having to implement a cleanup of our S3 infra in your product's own repo with To me this is an S3 and cost management concern (something we Apps Infra are the caretakers of). Technically for most product teams we don't expect them to have to think about S3 storage and storage costs and infrastructure impacts. So this is an infra concern, and should be transparent to product teams as we don't expect them to have to manage this or have to think about it, we do. So they shouldn't have to handle the burden of setting up a scheduled job (which will require to touch another repo to begin with anyway) then create a lane to handle and implement the logic determining what to clean up and when. Having to do that on their end would be mixing concerns IMHO. Besides, S3 Lifecycles are build explicitly for this, and have already all the logic built into it (including logic like allowing to keep only N versions of a file rather than / in addition to specifying a number of days, not requiring CI schedule jobs, etc), so creating an action for that then setting up a scheduled job etc feels like reinventing the wheel ourselves, instead of using a built-in feature of S3 that is made exactly for this at no extra cost and without requiring manual maintenance? |
I tend to agree, not much more to add -- I'd rather lean into a (almost) zero-maintenance, zero-code solution that's already battle tested... It's also less "operational surface area". A cron job + Fastlane action can silently break (expired credentials, CI error, etc). Lifecycle rules are declarative and managed by AWS. Aside from the cache use case, perhaps there is another one that'd make it worth to keep the action though? 🤔 (but then at this point we're looking for a problem with a given a solution 😅 ). |
Motivation
This action supports cleaning up S3 artifacts that we upload as part of CI — for example, the XCFramework snapshots published by Automattic/wordpress-rs#1199. Those artifacts are large and we don't want to store them indefinitely for no reason.
The prefix +
older_than_dayscombination lets us set up periodic cleanup jobs (e.g. "delete all PR build artifacts older than 180 days") so storage doesn't grow unbounded.Summary
delete_from_s3fastlane action that complements the existingupload_to_s3action:key) and bulk deletion by prefix (:prefix):older_than_days— only delete objects older than N days:dry_runmode to preview what would be deleted without making changes:silentflag to suppress itTest plan
bundle exec rspec spec/delete_from_s3_spec.rb— all 29 examples passbundle exec rubocop lib/fastlane/plugin/wpmreleasetoolkit/actions/common/delete_from_s3.rb spec/delete_from_s3_spec.rb— 0 offenses## Trunk/### New Features🤖 Generated with Claude Code