Skip to content

feat: add LZ4 compression in storage#55

Merged
christyjacob4 merged 9 commits intoutopia-php:mainfrom
aloks98:feat-3998-introduce-LZ4-compression-for-storage
Jan 18, 2023
Merged

feat: add LZ4 compression in storage#55
christyjacob4 merged 9 commits intoutopia-php:mainfrom
aloks98:feat-3998-introduce-LZ4-compression-for-storage

Conversation

@aloks98
Copy link
Copy Markdown

@aloks98 aloks98 commented Oct 2, 2022

I have added the LZ4 package for compression, but I cannot get over the fact that the compressed sizes are coming greater than the actual file sizes. Is there something I am missing? Can somebody please help me with this?

@stnguyen90
Copy link
Copy Markdown
Contributor

@aloks98 Thanks for the PR! 🤯 Please give us some time to review it.

@stnguyen90
Copy link
Copy Markdown
Contributor

Only local tests:

image

@stnguyen90
Copy link
Copy Markdown
Contributor

I cannot get over the fact that the compressed sizes are coming greater than the actual file sizes. Is there something I am missing? Can somebody please help me with this?

@aloks98, compressing doesn't always yield a smaller file, especially really small files, already compressed files, and files with totally random data. I tested your implementation on the RGB 8 bit version of the artificial image here and a file compressed to 13% of the original.

@stnguyen90
Copy link
Copy Markdown
Contributor

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

@stnguyen90 stnguyen90 linked an issue Oct 8, 2022 that may be closed by this pull request
6 tasks
Comment thread Dockerfile Outdated
Copy link
Copy Markdown
Contributor

@christyjacob4 christyjacob4 left a comment

Choose a reason for hiding this comment

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

Please address the comments

@aloks98
Copy link
Copy Markdown
Author

aloks98 commented Oct 15, 2022

hey @christyjacob4
I have updated the version to last commit hash

Copy link
Copy Markdown
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.

Sorry, would you please make some additional changes? It seems like the image is failing to build.

Comment thread Dockerfile Outdated
Comment thread Dockerfile Outdated
@aloks98
Copy link
Copy Markdown
Author

aloks98 commented Oct 19, 2022

Alright, got it!

@stnguyen90 stnguyen90 self-requested a review October 19, 2022 22:42
@stnguyen90
Copy link
Copy Markdown
Contributor

Thank you! 🙏

@christyjacob4
Copy link
Copy Markdown
Contributor

Please sync your branch with main . It has fixes for the failing CI tests

@christyjacob4
Copy link
Copy Markdown
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 :)

@christyjacob4 christyjacob4 merged commit 7ad182d into utopia-php:main Jan 18, 2023
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.

🗜️ Refine Appwrite Storage with LZ4 Compression

3 participants