-
Notifications
You must be signed in to change notification settings - Fork 0
Storage batch operation samples v2 #37
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
base: main
Are you sure you want to change the base?
Conversation
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.
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 introduces a new set of Ruby samples for Google Cloud Storage batch operations, covering common management tasks such as creating, listing, retrieving, canceling, and deleting batch jobs. These samples aim to provide clear, executable examples for developers interacting with the Storage Batch Operations API, ensuring proper usage and demonstrating key functionalities.
Highlights
- New Ruby Samples: This PR introduces a new set of Ruby samples for Google Cloud Storage batch operations.
- Batch Job Operations: The samples demonstrate how to create, list, retrieve, cancel, and delete batch jobs.
- Testing: Comprehensive acceptance tests are included to validate the functionality of the new samples.
- Configuration and Environment Setup: A dedicated environment for these samples is set up with specific Rubocop, Gemfile, and Rakefile configurations.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new set of samples for Google Cloud Storage batch operations. The overall structure is good, but I've found several critical issues, primarily in the command-line execution logic of the sample files, which would prevent them from running correctly. There is also a logic flaw in a test helper method. Addressing these points will significantly improve the quality and usability of these new samples.
| create_job( | ||
| bucket_name: ARGV.shift, | ||
| prefix: ARGV.shift, | ||
| job_name: ARGV.shift, | ||
| project_name: ARGV.shift, | ||
| job_type: ARGV.shift | ||
| ) |
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.
The create_job method is being called with a job_type keyword argument, but the method signature for create_job does not include this parameter. This will raise an ArgumentError when the script is run from the command line, making the sample unusable.
create_job(
bucket_name: ARGV.shift,
prefix: ARGV.shift,
job_name: ARGV.shift,
project_name: ARGV.shift
)| end | ||
| # [END storage_batch_list_job] | ||
|
|
||
| delete_job project_name: ARGV.shift, job_name: ARGV.shift if $PROGRAM_NAME == __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.
The command-line invocation for this sample incorrectly calls delete_job instead of list_job. It also passes a job_name argument, which list_job does not accept. This appears to be a copy-paste error that will cause the script to fail when run.
list_job project_name: ARGV.shift if $PROGRAM_NAME == __FILE__| 5.times do | ||
| if yield[/job_status-\s*(\w+)/, 1] == "RUNNING" | ||
| puts "Job in Running status. Gonna try again" | ||
| sleep rand(10..16) | ||
| end | ||
| end |
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.
The retry_job_status method's loop will always run 5 times, regardless of the job's status. If the job is not RUNNING, the loop continues without breaking, which is inefficient and may not reflect the intended waiting logic. The loop should terminate as soon as the job is no longer in a RUNNING state to avoid unnecessary iterations and delays.
5.times do
status = yield[/job_status-\s*(\w+)/, 1]
break unless status == "RUNNING"
puts "Job in Running status. Gonna try again"
sleep rand(10..16)
end| end | ||
| # [END storage_batch_get_job] | ||
|
|
||
| get_job project_name: ARGV.shift, job_name: ARGV.shift if $PROGRAM_NAME == __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.
When this script is executed from the command line, the get_job method is called, but its return value is not printed to the console. This means the user will see no output, which is not helpful. The returned string containing the job status should be printed.
puts get_job project_name: ARGV.shift, job_name: ARGV.shift if $PROGRAM_NAME == __FILE__| # The name of your Storage batch operation job | ||
| # job_name = "your-job-name" | ||
|
|
||
| require "google/cloud/storage_batch_operations" |
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.
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>