Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Commit

Permalink
Revert #2096 - [sites] Don't delete unused assets
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
threepointone committed Mar 8, 2022
1 parent 6e0737a commit bf829e7
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 9 deletions.
18 changes: 14 additions & 4 deletions src/commands/dev/edge/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,22 @@ pub(super) fn upload(
) -> Result<String> {
let client = crate::http::legacy_auth_client(user);

let asset_manifest = if let Some(site_config) = target.site.clone() {
let (to_delete, asset_manifest, site_namespace_id) = if let Some(site_config) =
target.site.clone()
{
let site_namespace = add_namespace(user, target, true)?;
let path = Path::new(&site_config.bucket);
let (to_upload, asset_manifest) = sync(target, user, &site_namespace.id, path)?;
let (to_upload, to_delete, asset_manifest) = sync(target, user, &site_namespace.id, path)?;

// First, upload all existing files in given directory
if verbose {
StdOut::info("Uploading updated files...");
}

bulk::put(target, user, &site_namespace.id, to_upload, &None)?;
Some(asset_manifest)
(to_delete, Some(asset_manifest), Some(site_namespace.id))
} else {
None
(Vec::new(), None, None)
};

let session_config = get_session_config(deploy_target);
Expand All @@ -56,6 +58,14 @@ pub(super) fn upload(
}
let response = response.error_for_status()?;

if !to_delete.is_empty() {
if verbose {
StdOut::info("Deleting stale files...");
}

bulk::delete(target, user, &site_namespace_id.unwrap(), to_delete, &None)?;
}

let text = &response.text()?;

// TODO: use cloudflare-rs for this :)
Expand Down
30 changes: 29 additions & 1 deletion src/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ pub fn publish(

let site_namespace = sites::add_namespace(user, target, false)?;

let (to_upload, asset_manifest) = sites::sync(target, user, &site_namespace.id, path)?;
let (to_upload, to_delete, asset_manifest) =
sites::sync(target, user, &site_namespace.id, path)?;

// First, upload all existing files in bucket directory
StdErr::working("Uploading site files");
Expand Down Expand Up @@ -104,6 +105,33 @@ pub fn publish(
upload::script(&upload_client, target, Some(asset_manifest))?;

run_deploy(target)?;

// Finally, remove any stale files
if !to_delete.is_empty() {
StdErr::info("Deleting stale files...");

let delete_progress_bar = if to_delete.len() > bulk::BATCH_KEY_MAX {
let delete_progress_bar = ProgressBar::new(to_delete.len() as u64);
delete_progress_bar.set_style(
ProgressStyle::default_bar().template("{wide_bar} {pos}/{len}\n{msg}"),
);
Some(delete_progress_bar)
} else {
None
};

bulk::delete(
target,
user,
&site_namespace.id,
to_delete,
&delete_progress_bar,
)?;

if let Some(pb) = delete_progress_bar {
pb.finish_with_message("Done deleting");
}
}
} else {
let upload_client = http::legacy_auth_client(user);

Expand Down
14 changes: 12 additions & 2 deletions src/preview/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ pub fn upload(
let site_namespace = add_namespace(user, target, true)?;

let path = Path::new(&site_config.bucket);
let (to_upload, asset_manifest) = sync(target, user, &site_namespace.id, path)?;
let (to_upload, to_delete, asset_manifest) =
sync(target, user, &site_namespace.id, path)?;

// First, upload all existing files in given directory
if verbose {
Expand All @@ -71,7 +72,16 @@ pub fn upload(

bulk::put(target, user, &site_namespace.id, to_upload, &None)?;

authenticated_upload(&client, target, Some(asset_manifest))?
let preview = authenticated_upload(&client, target, Some(asset_manifest))?;
if !to_delete.is_empty() {
if verbose {
StdOut::info("Deleting stale files...");
}

bulk::delete(target, user, &site_namespace.id, to_delete, &None)?;
}

preview
} else {
authenticated_upload(&client, target, None)?
}
Expand Down
18 changes: 16 additions & 2 deletions src/sites/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn sync(
user: &GlobalUser,
namespace_id: &str,
path: &Path,
) -> Result<(Vec<KeyValuePair>, AssetManifest)> {
) -> Result<(Vec<KeyValuePair>, Vec<String>, AssetManifest)> {
// First, find all changed files in given local directory (aka files that are now stale
// in Workers KV).

Expand All @@ -41,6 +41,20 @@ pub fn sync(
let (diff_files_to_upload, asset_manifest, _): (Vec<KeyValuePair>, AssetManifest, _) =
directory_keys_values(target, path, Some(&remote_keys))?;

// Now delete files from Workers KV that exist in remote but no longer exist locally.
// Get local keys
let mut local_keys: HashSet<_> = HashSet::new();
for (_, asset_key) in asset_manifest.iter() {
local_keys.insert(asset_key.clone());
}

// Find keys that are present in remote but not present in local, and
// stage them for deletion.
let to_delete: Vec<_> = remote_keys
.difference(&local_keys)
.map(|key| key.to_owned())
.collect();

StdErr::success("Success");
Ok((diff_files_to_upload, asset_manifest))
Ok((diff_files_to_upload, to_delete, asset_manifest))
}

0 comments on commit bf829e7

Please sign in to comment.