Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: support setting max_stream_count when fetching query result#2051

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Linchin merged 7 commits intogoogleapis:mainfromkien-truong:max-stream-count-api
Nov 22, 2024

Conversation

@kien-truong
Copy link
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-truongkien-truong requested review froma team ascode ownersNovember 3, 2024 03:29
@product-auto-labelproduct-auto-labelbot added size: sPull request size is small. api: bigqueryIssues related to the googleapis/python-bigquery API. labelsNov 3, 2024
@alvarowolfxalvarowolfx requested review fromchalmerlowe and removed request foralvarowolfxNovember 4, 2024 14:08
@LinchinLinchin added the kokoro:force-runAdd this label to force Kokoro to re-run the tests. labelNov 11, 2024
created by the server. If ``max_queue_size`` is :data:`None`, the queue
size is infinite.
max_stream_count (Optional[int]):
Copy link
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 ashere. It also mentions the effect ofpreserve_order (in this caseself._preserve_order), which I think we should make clear here.

Copy link
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.

Linchin reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Updated

.. versionadded:: 2.14.0
max_stream_count (Optional[int]):
Copy link
Contributor

Choose a reason for hiding this comment

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

@Linchin
Copy link
Contributor

Thank you@kien-truong for adding further support formax_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:runAdd this label to force Kokoro to re-run the tests. labelNov 11, 2024
@yoshi-kokoroyoshi-kokoro removed kokoro:runAdd this label to force Kokoro to re-run the tests. kokoro:force-runAdd this label to force Kokoro to re-run the tests. labelsNov 11, 2024
@kien-truong
Copy link
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 reacted with thumbs up emoji

@Linchin
Copy link
Contributor

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

@product-auto-labelproduct-auto-labelbot added size: mPull request size is medium. and removed size: sPull request size is small. labelsNov 16, 2024
@kien-truongkien-truongforce-pushed themax-stream-count-api branch 3 times, most recently from31662ad to0e52722CompareNovember 16, 2024 03:51
@kien-truong
Copy link
ContributorAuthor

I have added tests to cover user-providedmax_stream_count flow and fixed themypy test as well.

@chalmerlowechalmerlowe added the kokoro:runAdd this label to force Kokoro to re-run the tests. labelNov 18, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:runAdd this label to force Kokoro to re-run the tests. labelNov 18, 2024
@LinchinLinchin added the kokoro:runAdd this label to force Kokoro to re-run the tests. labelNov 18, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:runAdd this label to force Kokoro to re-run the tests. labelNov 18, 2024
@Linchin
Copy link
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:runAdd this label to force Kokoro to re-run the tests. labelNov 22, 2024
@yoshi-kokoroyoshi-kokoro removed the kokoro:runAdd this label to force Kokoro to re-run the tests. labelNov 22, 2024
@LinchinLinchin self-requested a reviewNovember 22, 2024 21:34
Copy link
Contributor

@LinchinLinchin left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@LinchinLinchin merged commitd461297 intogoogleapis:mainNov 22, 2024
17 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@LinchinLinchinLinchin approved these changes

@chalmerlowechalmerloweAwaiting requested review from chalmerlowe

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.

Makemax_stream_count configurable when using Bigquery Storage API

5 participants

@kien-truong@Linchin@alvarowolfx@chalmerlowe@yoshi-kokoro
[8]ページ先頭
©2009-2025 Movatter.jp