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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: `read_session` optional to `ReadRowsStream.rows()` #228

Merged

tswast merged 5 commits into googleapis:master from tswast:issue168-read_session

Jul 9, 2021

Contributor

tswast commented Jul 8, 2021

The schema from the first ReadRowsResponse message can be used to decode
messages, instead.

Note: to_arrow() and to_dataframe() do not work on an empty stream unless a
read_session has been passed in, as the schema is not available. This should
not affect google-cloud-bigquery and pandas-gbq, as those packages use the
lower-level message->dataframe/arrow methods.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #168 🦕


          feat: read_session optional to ReadRowsStream.rows()

378db09

The schema from the first `ReadRowsResponse` message can be used to decode
messages, instead.

Note: `to_arrow()` and `to_dataframe()` do not work on an empty stream unless a
`read_session` has been passed in, as the schema is not available. This should
not affect `google-cloud-bigquery` and `pandas-gbq`, as those packages use the
lower-level message->dataframe/arrow methods.

tswast requested a review from a team

July 8, 2021 19:19

tswast requested a review from a team as a code owner

July 8, 2021 19:19

tswast requested review from shollyman and removed request for a team

July 8, 2021 19:19

product-auto-label bot added the api: bigquerystorage label

google-cla bot added the cla: yes label


          revert change to comment

ffc5df2

This was referenced Jul 8, 2021

show deprecation warning when read_session passed to ReadRowsStream.rows, ReadRowsStream.to_arrow, ReadRowsStream.to_dataframe #229

Open

don't pass read_session to BQ Storage API rows from to_dataframe and to_arrow googleapis/python-bigquery#742

Closed

tseaver reviewed

View reviewed changes

google/cloud/bigquery_storage_v1/reader.py Outdated Show resolved Hide resolved

google/cloud/bigquery_storage_v1/reader.py Outdated Show resolved Hide resolved

google/cloud/bigquery_storage_v1/reader.py Outdated Show resolved Hide resolved

tests/system/conftest.py Show resolved Hide resolved

tests/system/conftest.py Show resolved Hide resolved


          use else for empty arrow streams in try-except block

0d693b3

Co-authored-by: Tres Seaver <[email protected]>

google-cla bot commented Jul 8, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

google-cla bot added cla: no and removed cla: yes labels

Contributor

tseaver commented Jul 8, 2021

@googlebot I consent.

google-cla bot commented Jul 8, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Contributor

tseaver commented Jul 8, 2021

@googlebot I fixed it.

google-cla bot commented Jul 8, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Contributor

tseaver commented Jul 8, 2021

@tswast I give up on the CLA bit.


          update docstring to reflect that readsession and readrowsresponse can…

dcb868b

… be used interchangeably

google-cla bot commented Jul 8, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Contributor Author

tswast commented Jul 8, 2021

@tswast I give up on the CLA bit.

No worries. I'll manually override the bot.


          update arrow deserializer, too

google-cla bot commented Jul 8, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

tswast added cla: yes and removed cla: no labels

tswast requested a review from tseaver

July 8, 2021 21:26

tseaver approved these changes

View reviewed changes

shollyman approved these changes

View reviewed changes

tests/unit/test_reader_v1.py

                   avro_blocks = _bq_to_avro_blocks([], avro_schema)
                   reader = class_under_test(avro_blocks, mock_gapic_client, "", 0, {})
+                  # Read session is needed to get a schema for empty streams.

Contributor

shollyman Jul 8, 2021

Do we think this is enough of a nuisance to request that empty streams always have at least one first response where the row data is null or empty?

Contributor Author

tswast Jul 9, 2021

Maybe? I'm not sure how often it'll come up now that they don't do automatic rebalancing.

I'm actually considering deprecating this method altogether, as it didn't turn out to be all that necessary. #230

tswast merged commit 4f56029 into googleapis:master

gcf-merge-on-green bot pushed a commit that referenced this pull request


          chore: release 2.6.0 (#227)

a309380

🤖 I have created a release \*beep\* \*boop\*
---
## [2.6.0](https://www.github.com/googleapis/python-bigquery-storage/compare/v2.5.0...v2.6.0) (2021-07-09)


### Features

* `read_session` optional to `ReadRowsStream.rows()` ([#228](https://www.github.com/googleapis/python-bigquery-storage/issues/228)) ([4f56029](https://www.github.com/googleapis/python-bigquery-storage/commit/4f5602950a0c1959e332aa2964245b9caf4828c8))
* add always_use_jwt_access ([#223](https://www.github.com/googleapis/python-bigquery-storage/issues/223)) ([fd82417](https://www.github.com/googleapis/python-bigquery-storage/commit/fd824174fb044fbacc83c647f619fda556333e26))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

tswast deleted the issue168-read_session branch

July 15, 2021 19:30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquerystorage cla: yes