Restart delete resumable upload v4 test #36
Conversation
Co-authored-by: Neha Bajaj <bajajneha27@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @shubhangi-google, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances the Google Cloud Storage Ruby client library by introducing new capabilities for managing resumable uploads. It adds methods to the Bucket class that allow users to restart interrupted uploads and explicitly delete pending resumable uploads. These additions provide greater control and flexibility over large file transfers, ensuring robustness and efficiency. The changes include updates to the core library and comprehensive test coverage for the new functionalities.
Highlights
- Resumable Upload Restart: A new method,
restart_resumable_upload, has been added to theBucketclass, enabling users to resume an interrupted file upload by providing the file and its unique upload ID. - Resumable Upload Deletion: A new method,
delete_resumable_upload, has been added to theBucketclass, allowing users to explicitly remove a pending resumable upload using its unique upload ID. - Service Layer Integration: Corresponding methods for both restarting and deleting resumable uploads have been implemented in the
Servicelayer to interact with the underlying Google Cloud Storage API. - Test Coverage: Comprehensive unit tests have been added for both
restart_resumable_uploadanddelete_resumable_uploadmethods, ensuring their correct functionality and proper error handling for missing arguments.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request adds functionality to restart and delete resumable uploads. The changes look good overall, introducing new methods on the Bucket and Service classes, along with corresponding tests. I've provided a few suggestions to improve documentation clarity and to simplify the new tests by removing redundant code. These changes will enhance maintainability and user experience.
| # (If the object does not have path, a `path` argument must be also be | ||
| # provided.) |
There was a problem hiding this comment.
The documentation for the file parameter is confusing. It mentions that a path argument must be provided if the file object doesn't have a path, but this method does not accept a path argument. This part of the documentation seems to be a leftover from a copy-paste and should be removed.
| # storage = Google::Cloud::Storage.new | ||
| # | ||
| # bucket = storage.bucket "my-bucket" | ||
| # bucket.restart_resumable_upload file,upload_id |
There was a problem hiding this comment.
For better readability and to adhere to common Ruby style, please add a space after the comma separating the arguments in the example.
# bucket.restart_resumable_upload file, upload_id|
|
||
| ## | ||
| # Delete resumable upload | ||
| # @param [String] upload_id Unique Id of an resumable upload |
There was a problem hiding this comment.
There's a small grammatical error in the documentation. It should be "a resumable upload" instead of "an resumable upload".
# @param [String] upload_id Unique Id of a resumable upload| it "raises ArgumentError if upload_id is not provided to restart_resumable_upload" do | ||
| new_file_name = random_file_path | ||
| upload_id = "TEST_ID" | ||
|
|
||
| Tempfile.open ["google-cloud", ".txt"] do |tmpfile| | ||
| tmpfile.write "Hello world" | ||
| tmpfile.rewind | ||
| mock = Minitest::Mock.new | ||
| mock.expect :restart_resumable_upload, create_file_gapi(bucket.name, new_file_name), | ||
| [bucket.name, tmpfile, upload_id], | ||
| **resumable_upload_args(options: {}) | ||
| bucket.service.mocked_service = mock | ||
| expect do | ||
| bucket.restart_resumable_upload tmpfile | ||
| end.must_raise ArgumentError | ||
| end | ||
| end |
There was a problem hiding this comment.
This test can be simplified. The mock setup is unnecessary because the ArgumentError is raised before the service method is called. The unused variables new_file_name and upload_id can also be removed. This makes the test simpler and more focused on what it's actually testing: the argument validation.
it "raises ArgumentError if upload_id is not provided to restart_resumable_upload" do
Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world"
tmpfile.rewind
expect do
bucket.restart_resumable_upload tmpfile
end.must_raise ArgumentError
end
end| it "raises ArgumentError if upload_id is not provided to delete_resumable_upload" do | ||
| upload_id = "TEST_ID" | ||
| mock = Minitest::Mock.new | ||
| mock.expect :delete_resumable_upload, true, | ||
| [bucket.name, upload_id], | ||
| **resumable_upload_args(options: {}) | ||
| bucket.service.mocked_service = mock | ||
| expect do | ||
| bucket.delete_resumable_upload | ||
| end.must_raise ArgumentError | ||
| end |
There was a problem hiding this comment.
Similar to the other argument error test, the mock setup here is unnecessary because the ArgumentError is raised before the service method is called. The test can be simplified by removing the mock and the unused upload_id variable.
it "raises ArgumentError if upload_id is not provided to delete_resumable_upload" do
expect do
bucket.delete_resumable_upload
end.must_raise ArgumentError
end
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>