impl(bigquery): fetch more pages and limit buffered results#6032
impl(bigquery): fetch more pages and limit buffered results#6032alvarowolfx wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements pagination support for the BigQuery RowIterator by implementing the fetch_page method using GetQueryResultsRequest, adding a set_max_rows_buffered configuration method, and adding corresponding unit and integration tests. The review feedback suggests avoiding the use of .expect() in fetch_page and instead returning a Result to prevent potential panics, aligning with the repository's style guide.
| let job_ref = self.job_ref.as_ref().expect( | ||
| "only queries with a job reference should have page tokens and can fetch more pages", | ||
| ); |
There was a problem hiding this comment.
According to the Repository Style Guide (lines 50-59), unwrap() and expect() should typically be avoided in production code, and the library should almost always prefer returning a Result over panicking. Since fetch_page returns a Result, we can safely return a RowError::InvalidRowFormat if job_ref is missing instead of panicking.
| let job_ref = self.job_ref.as_ref().expect( | |
| "only queries with a job reference should have page tokens and can fetch more pages", | |
| ); | |
| let job_ref = self.job_ref.as_ref().ok_or_else(|| { | |
| RowError::InvalidRowFormat("missing job reference for pagination".to_string()) | |
| })?; |
References
- unwrap() and expect() should typically be avoided in production code and examples (use ? or handle errors). The library should almost always prefer to return a Result<> over an internal panic!. (link)
There was a problem hiding this comment.
this is an unreachable state. Only happens if the backend runs a query as a stateless job (no job reference) and provide a page token (which is not possible)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6032 +/- ##
==========================================
+ Coverage 96.95% 96.98% +0.02%
==========================================
Files 248 248
Lines 62379 62500 +121
==========================================
+ Hits 60482 60616 +134
+ Misses 1897 1884 -13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Towards #5844