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

⚡ Add a Deno example to wipe an Appwrite bucket #40

Merged
merged 10 commits into from Dec 20, 2022

Conversation

Aragur
Copy link
Contributor

@Aragur Aragur commented Oct 4, 2022

This adds an example for Deno to wipe a complete Appwrite bucket.
This PR should close: appwrite/appwrite#4075

  • Function Payload

Screenshot 2022-10-04 at 23 47 54

  • Execution Log

Screenshot 2022-10-04 at 23 48 04

  • Proof that all files are deleted

Screenshot 2022-10-04 at 23 48 12

@Aragur Aragur changed the title Implement a Deno example to wipe an Appwrite bucket ⚡ Add a Deno example to wipe an Appwrite bucket Oct 4, 2022
@stnguyen90
Copy link
Contributor

@Aragur, thanks for the PR! 🤯 Please give us some time to review it.

deno/wipe_appwrite_bucket/appwrite.json Outdated Show resolved Hide resolved
deno/wipe_appwrite_bucket/src/mod.ts Outdated Show resolved Hide resolved
@Aragur Aragur requested a review from stnguyen90 October 5, 2022 09:15
deno/wipe_appwrite_bucket/README.md Show resolved Hide resolved
do {
listFiles = await storage.listFiles(bucketId, [sdk.Query.limit(limit), sdk.Query.offset(offset)]);
for(const file of listFiles.files) {
await storage.deleteFile(bucketId, file.$id);
Copy link
Contributor

Choose a reason for hiding this comment

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

There isn't a need to await every file delete. It would be a good tradeoff between performance and inundating the server if each "page" of files were all deleted concurrently before fetching the next "page".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, I removed it. Thanks :)

Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to delete every page concurrently rather than awaiting each delete. You can do something like:

while (true) {
  result = storage.listFiles();
  if (no results) break;
  
  promises = []
  result.files.forEach(file) {
    promise.push(storage.deleteFile(file);
  })
  
  try {
    await Promise.all(promises);
  } catch (e) {
    // handle accordingly
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot. I optimized the loop now :)

deno/wipe_appwrite_bucket/src/mod.ts Outdated Show resolved Hide resolved
@Aragur Aragur requested a review from stnguyen90 October 7, 2022 09:54
deno/wipe_appwrite_bucket/src/mod.ts Outdated Show resolved Hide resolved
@Aragur
Copy link
Contributor Author

Aragur commented Oct 13, 2022

@stnguyen90 So this is my 3rd attempt.
Now I'm looping over all the files. (This needs to be sync).
This will everytime delete all the files. And that's exactly what's required by this example..

do {
listFiles = await storage.listFiles(bucketId, [sdk.Query.limit(limit), sdk.Query.offset(offset)]);
for(const file of listFiles.files) {
await storage.deleteFile(bucketId, file.$id);
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to delete every page concurrently rather than awaiting each delete. You can do something like:

while (true) {
  result = storage.listFiles();
  if (no results) break;
  
  promises = []
  result.files.forEach(file) {
    promise.push(storage.deleteFile(file);
  })
  
  try {
    await Promise.all(promises);
  } catch (e) {
    // handle accordingly
  }
}

deno/wipe_appwrite_bucket/src/mod.ts Outdated Show resolved Hide resolved
@Aragur
Copy link
Contributor Author

Aragur commented Oct 17, 2022

@stnguyen90 Thanks a lot for your suggestion. Didn't even remember Promise.all(...) was a thing :)

Copy link
Contributor

@stnguyen90 stnguyen90 left a comment

Choose a reason for hiding this comment

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

Hey 👋 awesome work on your PR! We've approved your work and it'll be merged soon!

success: true,
});
}
const promises = listFiles.files.map((file) => storage.deleteFile(bucketId, file.$id));
Copy link
Contributor

Choose a reason for hiding this comment

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

nice usage of map!

@christyjacob4 christyjacob4 merged commit 579db4f into open-runtimes:main Dec 20, 2022
@christyjacob4
Copy link
Contributor

THANK YOU! All changes merged 🥳

Please reach out to me on our Discord server if you would like to claim your Appwrite swags! As a way of saying thank you, we would also love to invite you to join the Appwrite organization on GitHub. Please share your GitHub username with us on Discord.  

You can accept the invite by visiting https://github.com/orgs/appwrite/invitation. By joining our team, you will officially be an Appwrite maintainer on GitHub.

You can change your membership visibility settings, so your new Appwrite team membership badge will show up on your personal GitHub profile.

Please feel free to look for more PRs you might be interested in helping with on our long list of Hacktoberfest friendly issues and help make Appwrite better :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

⚡ Write a wipeAppwriteBucket() Function using Deno
3 participants