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

Remove empty Carrierwave directories when asset file is removed #385

Open
2 tasks
floehopper opened this issue Jan 8, 2018 · 9 comments
Open
2 tasks

Remove empty Carrierwave directories when asset file is removed #385

floehopper opened this issue Jan 8, 2018 · 9 comments

Comments

@floehopper
Copy link
Contributor

floehopper commented Jan 8, 2018

When we implemented #296, we didn't remove the directories as well as the files. This shouldn't cause any immediate problem, but it might be a bit confusing to people, so it would be good to stop the app leaving these directories lying around and to remove all the existing ones.

  • Change Asset Manager app so that it removes empty CarrierWave directories when a new asset file is uploaded to S3 and removed from the filesystem.
  • Remove empty CarrierWave directories for existing assets which have already been uploaded to S3 and deleted from the filesystem.
@floehopper
Copy link
Contributor Author

Note that for completeness, we may want to remove the empty directories from the govuk-attachments-production S3 bucket which prior to alphagov/govuk-puppet#7019 was sync-ing Asset Manager assets every night.

@floehopper
Copy link
Contributor Author

I've opened #390 to address this issue for new assets in order that the disk usage for Asset Manager assets doesn't increase and trigger the asset master/slave disk usage alert once alphagov/govuk-puppet#7016 has been applied and Asset Manager assets are no longer being sync-ed from asset master to asset slaves.

We will need to decide what to do about existing empty directories separately.

@floehopper floehopper self-assigned this Jan 10, 2018
@floehopper
Copy link
Contributor Author

I've just merged #390.

@floehopper
Copy link
Contributor Author

floehopper commented Jan 12, 2018

#390 has just been deployed to staging (@ ~10:35).

@floehopper
Copy link
Contributor Author

#390 has just been deployed to production (@ ~10:45).

@floehopper
Copy link
Contributor Author

We still need to remove the existing empty directories. I'm going to add a checklist to the description to make that more obvious.

@floehopper
Copy link
Contributor Author

I'm going to move this back in to the To Do column so we can decide when/if to prioritise the remaining work.

@floehopper
Copy link
Contributor Author

Since we've reverted #390 in #410, I'm going to uncheck the first checkbox in the description of this issue.

@floehopper
Copy link
Contributor Author

I'm going to un-assign myself from this issue since I'm no longer actively working on it.

@floehopper floehopper removed their assignment Jan 15, 2018
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