-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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 snappy compression #4336
Add snappy compression #4336
Conversation
@shtelzerartem , thanks for the PR! 🤯 Please give us some time to review it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I'm going to add the hacktoberfest-accepted
label, but the core team will need to clean up some things before this can be merged:
- Update API specs
- Bump utopia-php/storage in composer.json
- Fix merge conflict
…at-4002-add-snappy-comporession
}, | ||
{ | ||
"url": "https://github.com/shtelzerartem/storage.git", | ||
"type": "git" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder to remove this
<?php | ||
$home = $this->getParam('home', ''); | ||
$fileLimit = $this->getParam('fileLimit', 0); | ||
$fileLimitHuman = $this->getParam('fileLimitHuman', 0); | ||
$bucketPermissions = $this->getParam('bucketPermissions', null); | ||
$fileCreatePermissions = $this->getParam('fileCreatePermissions', null); | ||
$fileUpdatePermissions = $this->getParam('fileUpdatePermissions', null); | ||
?> | ||
|
||
<div | ||
data-service="storage.getBucket" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to remove this file. We no longer use it. You need to add the compression option here
https://github.com/appwrite/console/blob/72b3f26edb802517b8d1c9d02900dbb99da58d38/src/routes/console/project-%5Bproject%5D/storage/bucket-%5Bbucket%5D/settings/+page.svelte#L49
@@ -65,7 +66,7 @@ | |||
->param('enabled', true, new Boolean(true), 'Is bucket enabled?', true) | |||
->param('maximumFileSize', (int) App::getEnv('_APP_STORAGE_LIMIT', 0), new Range(1, (int) App::getEnv('_APP_STORAGE_LIMIT', 0)), 'Maximum file size allowed in bytes. Maximum allowed value is ' . Storage::human(App::getEnv('_APP_STORAGE_LIMIT', 0), 0) . '. For self-hosted setups you can change the max limit by changing the `_APP_STORAGE_LIMIT` environment variable. [Learn more about storage environment variables](docs/environment-variables#storage)', true) | |||
->param('allowedFileExtensions', [], new ArrayList(new Text(64), APP_LIMIT_ARRAY_PARAMS_SIZE), 'Allowed file extensions. Maximum of ' . APP_LIMIT_ARRAY_PARAMS_SIZE . ' extensions are allowed, each 64 characters long.', true) | |||
->param('compression', COMPRESSION_TYPE_NONE, new WhiteList([COMPRESSION_TYPE_NONE, COMPRESSION_TYPE_GZIP, COMPRESSION_TYPE_ZSTD]), 'Compression algorithm choosen for compression. Can be one of ' . COMPRESSION_TYPE_NONE . ', [' . COMPRESSION_TYPE_GZIP . '](https://en.wikipedia.org/wiki/Gzip), or [' . COMPRESSION_TYPE_ZSTD . '](https://en.wikipedia.org/wiki/Zstd), For file size above ' . Storage::human(APP_STORAGE_READ_BUFFER, 0) . ' compression is skipped even if it\'s enabled', true) | |||
->param('compression', 'none', new WhiteList([COMPRESSION_TYPE_NONE, COMPRESSION_TYPE_GZIP, COMPRESSION_TYPE_ZSTD, COMPRESSION_TYPE_SNAPPY]), 'Compression algorithm choosen for compression. Can be one of ' . COMPRESSION_TYPE_NONE . ', [' . COMPRESSION_TYPE_GZIP . '](https://en.wikipedia.org/wiki/Gzip), or [' . COMPRESSION_TYPE_ZSTD . '](https://en.wikipedia.org/wiki/Zstd), or [' . COMPRESSION_TYPE_SNAPPY . '](https://en.wikipedia.org/wiki/Snappy_(compression)), For file size above ' . Storage::human(APP_STORAGE_READ_BUFFER, 0) . ' compression is skipped even if it\'s enabled', true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace the string literals with the constants in all occurances in this file
There was a problem hiding this 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
@shtelzerartem thanks a lot for your contributions during Hacktoberfest 2022! 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. I'm closing this PR as there has not been any activity on this for quite a while. If you'd like to continue working on it , feel free to create a new PR. |
What does this PR do?
Adds Snappy compression adapter
Test Plan
Added 2 tests for Snappy compression similarly to ZSTD, both passed tests
testCreateBucketFileSnappyCompression
testFilePreviewSnappyCompression
Related PRs and Issues
Resolves #4002
Relates to my second PR #4002-utopia-php
Have you read the Contributing Guidelines on issues?
Yep :)