Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

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

Closed
emar-kar wants to merge 3 commits into
googleapis:mastergoogleapis/google-cloud-python:masterfrom
MaxxleLLC:iss-8475-(BigTable)MaxxleLLC/google-cloud-python:iss-8475-(BigTable)Copy head branch name to clipboard
Closed

feat(bigtable): add 'client_options' support to Client#9289
emar-kar wants to merge 3 commits into
googleapis:mastergoogleapis/google-cloud-python:masterfrom
MaxxleLLC:iss-8475-(BigTable)MaxxleLLC/google-cloud-python:iss-8475-(BigTable)Copy head branch name to clipboard

Conversation

@emar-kar

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

Copy link
Copy Markdown
Contributor

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

tseaver commented Oct 22, 2019

Copy link
Copy Markdown
Contributor

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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.