Skip to content

feat(bigtable): add 'client_options' support to Client #9289

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
emar-kar wants to merge 3 commits into from
Closed

feat(bigtable): add 'client_options' support to Client #9289

emar-kar wants to merge 3 commits into from

Conversation

@emar-kar
Copy link
Contributor

@emar-kar emar-kar commented Sep 25, 2019 •
edited by IlyaFaer
Loading

Towards #8475

List of changes:

  1. client:
    • as BigTable Client creates by GAPIC client_options were integrated into the _create_gapic_client method call.
  2. setup:
    • google-cloud-core version updated on the basis of the comment;
    • the file was reformatted by black.
  3. test_client:
    • since api_endpoint is an irrevocable argument, tests check the correctness of the API call;
    • by default client_optionsshould be None.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 25, 2019
@IlyaFaer IlyaFaer added api: bigtable Issues related to the Bigtable API. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Sep 25, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 25, 2019
@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 30, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 30, 2019
@emar-kar emar-kar marked this pull request as ready for review September 30, 2019 09:05
@tseaver tseaver mentioned this pull request Oct 22, 2019
Merged
@tseaver
Copy link
Contributor

tseaver commented Oct 22, 2019

This PR has several issues:

  • Bigtable has two endpoints, one for table data and one for table / instance admin, and therefore the client constructor needs both a client_options and an admin_client_options argument.
  • The correct client_options / admin_client_options arguments must be passed down to the GAPIC class, which means that _create_gapic_client must take that value as an argument, rather than looking it up on the instance.

PR #9516 corrects those two issues.

Also, please avoid intermingling janitorial changes (blackening setup.py) with substantive ones.

@tseaver tseaver closed this Oct 22, 2019
@emar-kar emar-kar deleted the iss-8475-(BigTable) branch October 22, 2019 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crwilcox crwilcox Awaiting requested review from crwilcox

@frankyn frankyn Awaiting requested review from frankyn

@tseaver tseaver Awaiting requested review from tseaver

Assignees

No one assigned

Labels

api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@emar-kar @tseaver @googlebot @IlyaFaer @yoshi-kokoro