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

move batching into bulk module code #1400

Merged
merged 3 commits into from
Jun 22, 2020

Conversation

ashleymichal
Copy link
Contributor

fixes #1398

@ashleymichal ashleymichal added status - needs review regression Something is broken, but works in previous releases sites labels Jun 19, 2020
@ashleymichal ashleymichal added this to the 1.10.2 milestone Jun 19, 2020
@ashleymichal ashleymichal requested a review from a team as a code owner June 19, 2020 19:00
@ashleymichal ashleymichal self-assigned this Jun 19, 2020
@@ -70,15 +69,15 @@ pub fn upload(
message::info("Uploading updated files...");
}

upload_files(target, user, &site_namespace.id, to_upload)?;
bulk::put(target, user, &site_namespace.id, to_upload, &None)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use a progress bar here? didn't before but can add.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make sense to me, right now it just says nothing and it might look like it's hanging

@ispivey ispivey requested a review from jspspike June 19, 2020 19:19
Copy link
Contributor

@jspspike jspspike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me and after testing I got good behavior.

 Built successfully, built project size is 12 KiB.
 Using namespace for Workers Site "__site-workers_sites_assets"
 Success
 Uploading site files
█████████████████████████████████████████████████████████████████████████████████████████████████████████████████ 11001/11001
Done Uploading
 Successfully published your script to https://site.ftc.workers.dev
 Deleting stale files...
█████████████████████████████████████████████████████████████████████████████████████████████████████████████████ 11001/11001
Done deleting

Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested this, it works and it looks like good programming

@EverlastingBugstopper EverlastingBugstopper merged commit 7873672 into master Jun 22, 2020
@delete-merged-branch delete-merged-branch bot deleted the bug/sites-bulk-delete branch June 22, 2020 20:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
regression Something is broken, but works in previous releases sites
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Critical: Error: Number of keys to delete (16010) exceeds max of 10000
3 participants