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
feat: add flag to allow disabling auto decompression by client #850
Conversation
af84737
to
1d785af
Compare
Codecov Report
@@ Coverage Diff @@
## master #850 +/- ##
==========================================
+ Coverage 95.26% 95.27% +<.01%
==========================================
Files 11 11
Lines 1205 1206 +1
Branches 300 300
==========================================
+ Hits 1148 1149 +1
Misses 29 29
Partials 28 28
Continue to review full report at Codecov.
|
src/file.ts
Outdated
@@ -359,6 +359,7 @@ export interface CreateReadStreamOptions { | |||
validation?: 'md5' | 'crc32c' | false | true; | |||
start?: number; | |||
end?: number; | |||
returnCompressed?: boolean; |
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.
How about decompress
, default 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.
done
src/file.ts
Outdated
@@ -1113,8 +1113,8 @@ class File extends ServiceObject<File> { | |||
* NOTE: Byte ranges are inclusive; that is, `options.start = 0` and | |||
* `options.end = 999` represent the first 1000 bytes in a file or object. | |||
* NOTE: when specifying a byte range, data integrity is not available. | |||
* @property {booblean} [returnCompressed] Disable auto decompression of the | |||
* received data. By default this option set to `false`. | |||
* @property {booblean} [decompress] Disable auto decompression of the |
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.
I think if you put [decompress=true]
, that'll tip off the docs site that the default value is true
. Also, I think it's just called a boolean
😅.
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.
Done! 😹
@@ -1120,6 +1121,10 @@ class File extends ServiceObject<File> { | |||
* NOTE: Byte ranges are inclusive; that is, `options.start = 0` and | |||
* `options.end = 999` represent the first 1000 bytes in a file or object. | |||
* NOTE: when specifying a byte range, data integrity is not available. | |||
* @property {boolean} [decompress=true] Disable auto decompression of the |
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.
Is this from the perspective of the GCS service?
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.
No, nodejs-sotage API always requests that the data should be served as is (compressed if applicable) i. e. sending appropriate headers
const headers = {
'Accept-Encoding': 'gzip',
'Cache-Control': 'no-store',
} as Headers;
This flag is only from the perspective of nodejs-storage client to allow this library to not transcode the data if received from the server in compressed form and return to user compressed..
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.
@frankyn ping!
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.
pending @jkwlui's last comment. Otherwise LGTM. Thanks for the ping.
@stephenplusplus No, the bottom line of #709 is that server does not respect This PR cover the case when user is downloading |
Fixes #827