omit accept-encoding from upload#59
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the handling of the Accept-Encoding: gzip header, moving it from a global service-level configuration to a specific per-request header for downloads. This change prevents potential parsing errors during file uploads where gzipped metadata might be returned. The PR includes updated documentation, new acceptance tests, and modified unit tests to verify the header's behavior. Feedback was provided to avoid in-place mutation of the options hash to prevent potential FrozenError exceptions.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request modifies the google-cloud-storage library to move the Accept-Encoding: gzip header from a global request option to a specific option for file downloads. This ensures that the header is omitted during file uploads while maintaining it for downloads. The changes include updates to the service initialization, the download_file method, documentation, and a comprehensive set of unit and acceptance tests to verify the new behavior. I have no feedback to provide.
shubhangi-google
force-pushed
the
remove_accept_encoding_gzip
branch
from
a523755 to
ca42944
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request removes the global Accept-Encoding: gzip header from the storage service initialization. Feedback indicates that this change could lead to performance regressions by disabling gzip compression for all API requests, including downloads, and suggests handling the header omission at the individual request level instead.
| @service.request_options.header ||= {} | ||
| @service.request_options.header["x-goog-api-client"] = | ||
| "gl-ruby/#{RUBY_VERSION} gccl/#{Google::Cloud::Storage::VERSION}" | ||
| @service.request_options.header["Accept-Encoding"] = "gzip" |
There was a problem hiding this comment.
Removing the Accept-Encoding: gzip header globally from the service initialization will disable gzip compression for all API requests, including downloads. This may lead to a performance regression due to increased bandwidth usage for download operations. If the intent is to omit this header specifically for uploads, it should be handled at the individual request level rather than globally.
2 participants
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
bundle exec rake ciin the gem subdirectory.closes: #<issue_number_goes_here>