impl(bigquery): add query polling and complete query#5896
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements query polling capabilities in the BigQuery crate, introducing the Query::until_done method, a CompleteQuery handle, and a poll_query_results helper function that uses exponential backoff. It also adds comprehensive unit tests to verify these behaviors. The review feedback points out violations of the repository style guide regarding the use of unwrap() and expect() in production code within the polling helper, recommending proper error propagation instead.
| let backoff_policy = ExponentialBackoffBuilder::default() | ||
| .with_maximum_delay(std::time::Duration::from_secs(10)) | ||
| .build() | ||
| .unwrap(); |
There was a problem hiding this comment.
According to the Repository Style Guide, unwrap() should not be used in production code. Instead of panicking on failure, handle the error by mapping it to a QueryError and propagating it using the ? operator.
let backoff_policy = ExponentialBackoffBuilder::default()
.with_maximum_delay(std::time::Duration::from_secs(10))
.build()
.map_err(|e| {
QueryError::Rpc {
source: google_cloud_gax::error::Error::service(
google_cloud_gax::error::rpc::Status::default()
.set_code(google_cloud_gax::error::rpc::Code::Internal)
.set_message(format!("failed to build backoff policy: {e}")),
),
}
})?;References
- No unwrap() or expect() in production code or examples (use ? or handle errors). (link)
| let mut state = PollingState::default(); | ||
|
|
||
| loop { | ||
| let job_ref = job_ref.expect("query job should have job reference at this point"); |
There was a problem hiding this comment.
According to the Repository Style Guide, expect() should not be used in production code. Instead of panicking if the job reference is missing, return a QueryError explicitly.
| let job_ref = job_ref.expect("query job should have job reference at this point"); | |
| let job_ref = job_ref.ok_or_else(|| { | |
| QueryError::Rpc { | |
| source: google_cloud_gax::error::Error::service( | |
| google_cloud_gax::error::rpc::Status::default() | |
| .set_code(google_cloud_gax::error::rpc::Code::InvalidArgument) | |
| .set_message("query job should have job reference at this point"), | |
| ), | |
| } | |
| })?; |
References
- No unwrap() or expect() in production code or examples (use ? or handle errors). (link)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5896 +/- ##
========================================
Coverage 97.89% 97.90%
========================================
Files 233 234 +1
Lines 59202 59432 +230
========================================
+ Hits 57958 58189 +231
+ Misses 1244 1243 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Towards #5844