impl(bigquery): add default job retry policies - #6233
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new retry policy module (retry_policy.rs) for BigQuery, defining the RetryableErrors policy and helper functions to map BigQuery error reasons to gRPC status codes. Feedback points out a logic issue in query_job_failed_to_gax_error where permanent errors could be incorrectly mapped to a retryable Code::Unavailable status, potentially causing unnecessary retries. A code suggestion is provided to prioritize retryable errors and fall back to a non-retryable code if none are found.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6233 +/- ##
==========================================
- Coverage 96.07% 96.06% -0.01%
==========================================
Files 275 276 +1
Lines 68829 69003 +174
==========================================
+ Hits 66126 66289 +163
- Misses 2703 2714 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a retry policy for BigQuery, defining retry strategies for both RPC and job-level execution. The review feedback identifies several critical issues: a private-in-public visibility mismatch for RetryableErrors, a type mismatch and logical bug in is_retryable_errors (which should use .all() instead of .any() to avoid retrying permanent failures), a test failure in test_job_attempt_limit due to a mismatched attempt limit, and an outdated assertion in test_is_retryable_errors.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces retry and backoff policies for BigQuery, including RetryableErrors for RPC retries and RetryableJobErrors for job-level retries. Feedback suggests updating the RetryableErrors policy to check state.idempotent before retrying to prevent unsafe retries on non-idempotent requests, and updating the corresponding unit tests to verify both idempotent and non-idempotent retry behaviors.
Towards #6218 #5844