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

BigQuery: Add support of the project ID with org prefix to the Table.from_string() method #9161

Conversation

emar-kar
Copy link
Contributor

@emar-kar emar-kar commented Sep 3, 2019

ID splitting was moved to a separate method to avoid code duplication. Dataset file changed to the new implementation.

Closes: #7827

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 3, 2019
@IlyaFaer IlyaFaer added api: bigquery Issues related to the BigQuery API. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Sep 3, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 3, 2019
flake8 fixed
@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 3, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 3, 2019
@IlyaFaer IlyaFaer requested review from tswast and plamut September 3, 2019 12:49
@IlyaFaer IlyaFaer marked this pull request as ready for review September 3, 2019 12:50
@IlyaFaer IlyaFaer requested review from a team and removed request for a team September 3, 2019 12:50
Copy link
Contributor

@plamut plamut left a comment

Choose a reason for hiding this comment

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

The PR fixes the reported issue. 👍

There is one docstring missing, and the regex pattern might need a change, please check the comment if it applies.

BTW, it would probably be useful to add another test case for tables with an optional partition suffix, e.g. "example.com:project_id.dataset_id.table_id$20190903"
.

bigquery/google/cloud/bigquery/_helpers.py Show resolved Hide resolved
bigquery/google/cloud/bigquery/_helpers.py Show resolved Hide resolved
* added the docstring for the '_split_id' method
Copy link
Contributor

@plamut plamut left a comment

Choose a reason for hiding this comment

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

Under the assumption that we do not want to handle all possible cases such as too long ID parts, etc., these changes look good to me. 👍

@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 5, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 5, 2019
@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 5, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 5, 2019
@plamut plamut merged commit 9f471fb into googleapis:master Sep 6, 2019
@emar-kar emar-kar deleted the project-ID-w-prefix-for-the-Table.from_string-method branch September 9, 2019 07:14
emar-kar added a commit to MaxxleLLC/google-cloud-python that referenced this pull request Sep 11, 2019
…from_string() method (googleapis#9161)

* add prefix support

* Update _helpers.py

* consolidate the regex

* update split_id method

* '_parse_id' method renamed to '_split_id'
* switched to 're.groups' implementation instead of partly grouping

* Update dataset.py

flake8 fixed

* Update _helpers.py

* added the docstring for the '_split_id' method

* fix lint failure
emar-kar added a commit to MaxxleLLC/google-cloud-python that referenced this pull request Sep 18, 2019
…from_string() method (googleapis#9161)

* add prefix support

* Update _helpers.py

* consolidate the regex

* update split_id method

* '_parse_id' method renamed to '_split_id'
* switched to 're.groups' implementation instead of partly grouping

* Update dataset.py

flake8 fixed

* Update _helpers.py

* added the docstring for the '_split_id' method

* fix lint failure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BigQuery: Table.from_string() doesn't support GCP project ID with org prefix
6 participants