Skip to content

refactor: update insert_rows.py for more efficient streaming #212

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
Closed
jmgreger wants to merge 5 commits into from
Closed

refactor: update insert_rows.py for more efficient streaming #212

jmgreger wants to merge 5 commits into from

Conversation

@jmgreger
Copy link

@jmgreger jmgreger commented Aug 3, 2020

Added in the use of selected_fields=schema to explicitly define the schema. This prevents excessive calling of table metadata when streaming large volumes of data.

There may be a better way to showcase this in the code, but I wanted to put this out there for review.

Fixes #211 🦕

@jmgreger jmgreger requested a review from shollyman as a code owner August 3, 2020 16:39
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 3, 2020
@HemangChothani HemangChothani changed the title Update insert_rows.py for more efficient streaming refactor: update insert_rows.py for more efficient streaming Aug 4, 2020
@plamut plamut added the type: docs Improvement to the documentation for an API. label Aug 4, 2020
@release-please release-please bot requested a review from a team as a code owner August 4, 2020 16:39
@release-please release-please bot requested review from leahecole and removed request for a team August 4, 2020 16:39
tseaver
tseaver suggested changes Aug 5, 2020
View reviewed changes
Copy link
Contributor

@tseaver tseaver left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please rebase to remove the release-related changes from this PR (the first three commits). Oops, my bad: the issue is that your PR targets the release-v1.27.0 branch, instead of master.

@tseaver tseaver added do not merge Indicates a pull request not ready for merge, due to either quality or timing. and removed type: docs Improvement to the documentation for an API. labels Aug 5, 2020
@jmgreger
Copy link
Author

jmgreger commented Aug 11, 2020

Please rebase to remove the release-related changes from this PR (the first three commits). Oops, my bad: the issue is that your PR targets the release-v1.27.0 branch, instead of master.

Ahh, sorry about that. I don't submit code very often :/. Should I make a new PR against master instead?

@jmgreger jmgreger changed the base branch from release-v1.27.0 to master August 11, 2020 02:33
@jmgreger jmgreger requested a review from tseaver August 11, 2020 02:36
@jmgreger
Copy link
Author

jmgreger commented Aug 11, 2020

I re-based this off of the master branch, now it just needs the files added by the chore bot to be removed. Doesn't look like I have rights to do that though.

@tseaver
Copy link
Contributor

tseaver commented Aug 11, 2020

@jmgreger If you use git rebase -i in your local clone, you should be able to delete the bot-generated commits. E.g.:

$ git checkout master
$ git fetch --all upstream  #  assuming this repo's remote is named 'upstream'.
$ git pull upstream/master
$ git checkout patch-1
$ git rebase -i master  # delete first three commits and the last merge commit
$ git push -f origin patch-1

As an alternative, you could just cherry-pick the patch from this commit into a new branch against current master, and then submit a new PR from that branch.

@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Aug 21, 2020
tswast
tswast requested changes Aug 24, 2020
View reviewed changes
samples/table_insert_rows.py
rows_to_insert = [(u"Phred Phlyntstone", 32), (u"Wylma Phlyntstone", 29)] #populate data for entry

try:
if 'schema' not in globals():
Copy link
Contributor

@tswast tswast Aug 24, 2020

Choose a reason for hiding this comment

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

Please keep the sample simple. This is unnecessary branching. If anything, we can call out that the schema can be fetched from a table in a comment.

samples/table_insert_rows.py
client.insert_rows(table_id, selected_fields=schema, rows_to_insert) # Stream data to BQ
print("New rows have been added.")
except ValueError:
print("Table’s schema is not set or rows is not a Sequence.")
Copy link
Contributor

@tswast tswast Aug 24, 2020

Choose a reason for hiding this comment

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

No need to catch this ValueError. Just let the exception bubble up.

@leahecole
Copy link
Contributor

leahecole commented Aug 24, 2020

LGTM once Tim's comments are addressed and as long as tests pass

jmgreger and others added 4 commits August 31, 2020 15:52
@jmgreger
Update insert_rows.py for more efficient streaming
060419c 
Added in the use of selected_fields=schema to explicitly define the schema. This prevents excessive calling of table metadata when streaming large volumes of data.

There may be a better way to showcase this in the code, but I wanted to put this out there for review.
testing: split system tests into separate builds (googleapis#207)
9ceeafc
@renovate-bot @tseaver
chore(deps): pin ipython to v7.17.0 for samples, python >= 3.7 (googl…
023b513 
…eapis#208)

Co-authored-by: Tres Seaver <[email protected]>
Co-authored-by: Takashi Matsuo <[email protected]>
@tseaver
chore: fix docs build broken by Sphinx 3.2.0 (googleapis#218)
970d839 
Closes googleapis#217.
@google-cla
Copy link

google-cla bot commented Aug 31, 2020

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Aug 31, 2020
@google-cla
Copy link

google-cla bot commented Aug 31, 2020

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@jmgreger jmgreger mentioned this pull request Aug 31, 2020
Closed
@jmgreger jmgreger mentioned this pull request Aug 31, 2020
Closed
@plamut
Copy link
Contributor

plamut commented Sep 1, 2020

Closing this in favor of #253 as per comment in that PR.

@plamut plamut closed this Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tswast tswast tswast requested changes

@shollyman shollyman Awaiting requested review from shollyman

@leahecole leahecole Awaiting requested review from leahecole

@tseaver tseaver Awaiting requested review from tseaver

Assignees

No one assigned

Labels

api: bigquery Issues related to the googleapis/python-bigquery API. cla: no This human has *not* signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve guidance for streaming inserts

6 participants

@jmgreger @tseaver @leahecole @plamut @tswast @renovate-bot