Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

feat: support setting max_stream_count when fetching query result - #2051

Merged
Linchin merged 7 commits into
googleapis:mainfrom
kien-truong:max-stream-count-api
Nov 22, 2024
Merged

feat: support setting max_stream_count when fetching query result#2051
Linchin merged 7 commits into
googleapis:mainfrom
kien-truong:max-stream-count-api

Conversation

@kien-truong

@kien-truongkien-truong commented Nov 3, 2024

Copy link
Copy Markdown
Contributor

Allow user to set max_stream_count when fetching result using BigQuery Storage API with incremental methods:

  • to_arrow_iterable
  • to_dataframe_iterable

Fixes#2030 🦕

@kien-truong
kien-truong requested review from a team and alvarowolfxNovember 3, 2024 03:29
@product-auto-labelproduct-auto-labelBot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/python-bigquery API. labels Nov 3, 2024
@alvarowolfx
alvarowolfx requested review from chalmerlowe and removed request for alvarowolfxNovember 4, 2024 14:08
@LinchinLinchin added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 11, 2024
@LinchinLinchin assigned Linchin and unassigned alvarowolfxNov 11, 2024
Linchin
Linchin reviewed Nov 11, 2024
View reviewed changes
Comment threadgoogle/cloud/bigquery/table.py
created by the server. If ``max_queue_size`` is :data:`None`, the queue
size is infinite.

max_stream_count (Optional[int]):

@LinchinLinchinNov 11, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more consistent if we use the same docstring as here. It also mentions the effect of preserve_order (in this case self._preserve_order), which I think we should make clear here.

@kien-truongkien-truongNov 11, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, _preserve_order is automatically set by parsing the queries, and not a user-facing API. I'll update the docstring to mention that effect.

@kien-truongkien-truongNov 16, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Linchin
Linchin reviewed Nov 11, 2024
View reviewed changes
Comment threadgoogle/cloud/bigquery/table.py

.. versionadded:: 2.14.0

max_stream_count (Optional[int]):

@LinchinLinchinNov 11, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as https://github.com/googleapis/python-bigquery/pull/2051/files#r1836568949

@Linchin

Linchin commented Nov 11, 2024

Copy link
Copy Markdown
Contributor

Thank you @kien-truong for adding further support for max_stream_count! I left some comments, and I wonder if you could add some unit tests too (we typically require 100% unit test coverage). You could find similar tests in PR #2039. If you need any help with it, feel free to let me know. I'd be more than glad to help!

@LinchinLinchin added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 11, 2024
@yoshi-kokoroyoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Nov 11, 2024
@kien-truong

kien-truong commented Nov 11, 2024

Copy link
Copy Markdown
ContributorAuthor

Hi, the default code path with the default arguments is already covered by the current tests.
I'll see if I can add tests for when the user manually set the argument when I have time at weekend.

@Linchin

Linchin commented Nov 11, 2024

Copy link
Copy Markdown
Contributor

@kien-truong sounds good. The mypy test is also failing, could you fix it too? You can run it by running nox -s mypy in the repo. (You can install nox by pip install nox)

@product-auto-labelproduct-auto-labelBot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Nov 16, 2024
@kien-truong
kien-truongforce-pushed the max-stream-count-api branch 3 times, most recently from 31662ad to 0e52722CompareNovember 16, 2024 03:51
@kien-truong

kien-truong commented Nov 16, 2024

Copy link
Copy Markdown
ContributorAuthor

I have added tests to cover user-provided max_stream_count flow and fixed the mypy test as well.

@kien-truongkien-truong mentioned this pull request Nov 16, 2024
Closed
@chalmerlowechalmerlowe added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
@LinchinLinchin added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 18, 2024
Linchin
Linchin reviewed Nov 19, 2024
View reviewed changes
Comment threadtests/unit/test_table.py Outdated
Linchin
Linchin reviewed Nov 19, 2024
View reviewed changes
Comment threadtests/unit/test_table.py Outdated
@Linchin

Linchin commented Nov 19, 2024

Copy link
Copy Markdown
Contributor

Thanks @kien-truong, the PR mostly looks good. I just have some small question regarding ignored coverage for the tests.

@LinchinLinchin added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 22, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 22, 2024
@Linchin
Linchin self-requested a review November 22, 2024 21:34
Linchin
Linchin approved these changes Nov 22, 2024
View reviewed changes

@LinchinLinchin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

@Linchin
Linchin merged commit d461297 into googleapis:mainNov 22, 2024
@release-pleaserelease-pleaseBot mentioned this pull request Nov 18, 2024
Merged
@chalmerlowechalmerlowe mentioned this pull request Nov 25, 2024
Closed
This was referenced Jan 21, 2025
Closed
Closed
Closed
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@chalmerlowechalmerloweAwaiting requested review from chalmerlowe
1 more reviewer
@LinchinLinchinLinchin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@LinchinLinchin

Labels

api: bigqueryIssues related to the googleapis/python-bigquery API.size: mPull request size is medium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make max_stream_count configurable when using Bigquery Storage API

5 participants

@kien-truong@Linchin@alvarowolfx@chalmerlowe@yoshi-kokoro