Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: recommend insert_rows_json to avoid call to tables.get #258

Merged
merged 4 commits into from Sep 15, 2020

Conversation

@tswast
Copy link
Contributor

@tswast tswast commented Sep 11, 2020

Since tables.get has a much lower QPS than tabledata.insertAll, we want
to avoid recommending a pattern that requires fetching a table schema.
If developers convert to a dictionary of the correct JSON format, no
table schema is required.

Closes #253
Closes #211

Since tables.get has a much lower QPS than tabledata.insertAll, we want
to avoid recommending a pattern that requires fetching a table schema.
If developers convert to a dictionary of the correct JSON format, no
table schema is required.
@tswast tswast requested review from shollyman and as code owners Sep 11, 2020
@google-cla google-cla bot added the cla: yes label Sep 11, 2020
@tswast tswast requested a review from plamut Sep 11, 2020
@tswast
Copy link
Contributor Author

@tswast tswast commented Sep 11, 2020

@jmgreger Please take a look. In my opinion this is a clearer / simpler way to solve #211 and avoid extra calls to tables.get while streaming.

Loading

Copy link

@jmgreger jmgreger left a comment

I like it, definitely cleaner than my version!

Loading

@tswast tswast requested a review from steffnay Sep 14, 2020
Copy link
Contributor

@shollyman shollyman left a comment

minor comment nits

Loading

@@ -25,12 +24,16 @@ def table_insert_rows_explicit_none_insert_ids(table_id):
# TODO(developer): Set table_id to the ID of the model to fetch.
Copy link
Contributor

@shollyman shollyman Sep 14, 2020

Choose a reason for hiding this comment

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

Comment update here as well

Loading

@@ -25,10 +24,14 @@ def table_insert_rows(table_id):
# TODO(developer): Set table_id to the ID of the model to fetch.
Copy link
Contributor

@shollyman shollyman Sep 14, 2020

Choose a reason for hiding this comment

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

Comment needs updated, there's no fetching.

Loading

Copy link
Contributor Author

@tswast tswast Sep 14, 2020

Choose a reason for hiding this comment

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

Thanks. Good catch

Loading

@gcf-merge-on-green
Copy link

@gcf-merge-on-green gcf-merge-on-green bot commented Sep 15, 2020

Your PR has attempted to merge for 3 hours. Please check that all required checks have passed, you have an automerge label, and that all your reviewers have approved the PR

Loading

@gcf-merge-on-green
Copy link

@gcf-merge-on-green gcf-merge-on-green bot commented Sep 15, 2020

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

Loading

@tswast tswast merged commit ae647eb into googleapis:master Sep 15, 2020
10 checks passed
Loading
@tswast tswast deleted the pr253-insert-rows-json branch Sep 15, 2020
gcf-merge-on-green bot pushed a commit that referenced this issue Sep 22, 2020
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.28.0](https://www.github.com/googleapis/python-bigquery/compare/v1.27.2...v1.28.0) (2020-09-22)


### Features

* add custom cell magic parser to handle complex `--params` values ([#213](https://www.github.com/googleapis/python-bigquery/issues/213)) ([dcfbac2](https://www.github.com/googleapis/python-bigquery/commit/dcfbac267fbf66d189b0cc7e76f4712122a74b7b))
* add instrumentation to list methods ([#239](https://www.github.com/googleapis/python-bigquery/issues/239)) ([fa9f9ca](https://www.github.com/googleapis/python-bigquery/commit/fa9f9ca491c3f9954287102c567ec483aa6151d4))
* add opentelemetry tracing ([#215](https://www.github.com/googleapis/python-bigquery/issues/215)) ([a04996c](https://www.github.com/googleapis/python-bigquery/commit/a04996c537e9d8847411fcbb1b05da5f175b339e))
* expose require_partition_filter for hive_partition ([#257](https://www.github.com/googleapis/python-bigquery/issues/257)) ([aa1613c](https://www.github.com/googleapis/python-bigquery/commit/aa1613c1bf48c7efb999cb8b8c422c80baf1950b))


### Bug Fixes

* fix dependency issue in fastavro ([#241](https://www.github.com/googleapis/python-bigquery/issues/241)) ([2874abf](https://www.github.com/googleapis/python-bigquery/commit/2874abf4827f1ea529519d4b138511d31f732a50))
* update minimum dependency versions ([#263](https://www.github.com/googleapis/python-bigquery/issues/263)) ([1be66ce](https://www.github.com/googleapis/python-bigquery/commit/1be66ce94a32b1f924bdda05d068c2977631af9e))
* validate job_config.source_format in load_table_from_dataframe ([#262](https://www.github.com/googleapis/python-bigquery/issues/262)) ([6160fee](https://www.github.com/googleapis/python-bigquery/commit/6160fee4b1a79b0ea9031cc18caf6322fe4c4084))


### Documentation

* recommend insert_rows_json to avoid call to tables.get ([#258](https://www.github.com/googleapis/python-bigquery/issues/258)) ([ae647eb](https://www.github.com/googleapis/python-bigquery/commit/ae647ebd68deff6e30ca2cffb5b7422c6de4940b))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants