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

delete-removed can not work (reliably) as expected #196

Open
tillkolter opened this issue Mar 16, 2024 · 0 comments
Open

delete-removed can not work (reliably) as expected #196

tillkolter opened this issue Mar 16, 2024 · 0 comments

Comments

@tillkolter
Copy link

tillkolter commented Mar 16, 2024

To whom it may concern.

I came across this just last weeks – three months after joining a new project realizing, that our S3 grew to an unreasonable size, containing assets from the first deployments.

The output on the runner seemed fine at first glance, printing output lines like this

Deleting files: [
  'assets/abc.js',
  'assets/efgs.js,
  ...
]
 
Deleting chunk: 1000
Deleting chunk: 916

<Next Job output>

Yet, the S3 still contained those files which were supposed to be deleted.

Unfortunately the upstream package s3-deploy seems to be removed from Github with the last publish 5 years. Not a good sign a should be a red flag for a package like the one in the repo here, which seems still to be having active commits.

Anyway, I looked into the code of the upstream package available here and it seems that there a two things wrong with the implementation

  1. call for deleteRemoved(s3Client, options.globbedFiles, options) is missing a yield, I guess this will lead to an early return from the process, leaving many files un-deleted
  2. even with the yield in front of the call, the Promise in deleteRemoved is resolving after the first batch, so I don't see how this should work.

I assume this Github action functionality was tested, but maybe with a smaller set of items, maybe this issue becomes only apparent after accumulating in the thousands? For our case it definitely doesn't work and understandably so, when looking at that library. I will probably implement this myself with simple AWS cli commands, because for our SPA we don't expect more than a page full of files.

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

No branches or pull requests

1 participant