[sites] Don't delete unused assets #2096
Conversation
We have a consistency issue when after uploading a new site, it takes a while for the worker to propagate, so incoming requests will try to fetch older assets, and 404. This isn't an issue just with us, it's a common scenario. besides, there's no real reason to delete older assets, we don't have limits on how many keys we use (https://developers.cloudflare.com/workers/platform/limits#kv-limits). Maybe someone could write a script that does a cleanup, but there's no reson to delete previous deployments assets immediately. (This matches Pages behaviour too). This PR removes the functionality where we delete unused assets on fresh uploads.
via `npm audit fix`
|
Storage is cheap but we still need some way of deleting unused assets eventually. If you don't delete them here (or overwrite them with an expiration time), how will you identify them later for deletion? |
|
Honestly I don't think we should delete static assets by default. It's fine. In my experience, it's incredibly rare that folks will keep uploading large fresh assets AND not reference previous older assets. Further, with what we're planning separately in other projects, it becomes even more important to keep previously built assets around. And finally, this is a temporary solution until we migrate sites to Pages. So I think it's ok. |
|
🎉 thanks for that.. we were getting occational problems due to missing asssets (e.g. dynamic lazy-loaded hash-stamped JS modules not existing anymore on server side), so I think the delete action on publish was naive to begin with. |
This reverts the changes made in #2096 that would preserve already uploaded assets with `[site]`, even if they weren't being used. This lads to longer upload times for sites with rapidly changing assets, and a burgeoning kv store. We're reverting this for now, and will add a proper fix later that expires unused assets on upload.
This reverts the changes made in #2096 that would preserve already uploaded assets with `[site]`, even if they weren't being used. This lads to longer upload times for sites with rapidly changing assets, and a burgeoning kv store. We're reverting this for now, and will add a proper fix later that expires unused assets on upload.
We have a consistency issue when after uploading a new site, it takes a while for the worker to propagate, so incoming requests will try to fetch older assets, and 404. This isn't an issue just with us, it's a common scenario. Besides, there's no real reason to delete older assets, we don't have limits on how many keys we use (https://developers.cloudflare.com/workers/platform/limits#kv-limits). Maybe someone could write a script that does a cleanup, but there's no reason to delete previous deployments assets immediately. (This matches Pages behaviour too).
This PR removes the functionality where we delete unused assets on fresh uploads.
(This is my first rust PR! Please verify I didn't screw anything up!)