Skip to content

BigQuery: Add client_options to constructors for manual clients. - #8999

Merged
busunkim96 merged 6 commits into
googleapis:masterfrom
MaxxleLLC:add-client_options-to-bigquery-constructor
Aug 27, 2019
Merged

BigQuery: Add client_options to constructors for manual clients.#8999
busunkim96 merged 6 commits into
googleapis:masterfrom
MaxxleLLC:add-client_options-to-bigquery-constructor

Conversation

@emar-kar

Copy link
Copy Markdown
Contributor

Toward #8475

@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 9, 2019
@emar-karemar-kar changed the title Add client options to bigquery constructorBigQuery: Add client_options to constructors for manual clients.Aug 9, 2019
@IlyaFaerIlyaFaer added api: bigquery Issues related to the BigQuery API. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 9, 2019
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 9, 2019
@IlyaFaer
IlyaFaer marked this pull request as ready for review August 9, 2019 09:48
@IlyaFaer
IlyaFaer requested review from a team, tseaver and tswastAugust 9, 2019 09:48
Comment threadbigquery/tests/unit/test_client.py Outdated
project=self.PROJECT,
credentials=creds,
_http=http,
client_options={"api_endpoint": Connection.DEFAULT_API_ENDPOINT},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Because this test is for default Client constructor arguments, let's omit client_options from this test, but still check that API_BASE_URL has the expected value.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see that we need a different test where we set client_options as a dictionary. Let's add such a test and set a value for api_endpoint to something other than the default value.

Comment threadbigquery/tests/unit/test_client.py Outdated

creds = _make_credentials()
http = object()
client_options = ClientOptions(Connection.DEFAULT_API_ENDPOINT)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please check with an alternative value for the api_endpoint, since we are trying to test for non-default values in this test.

Comment threadbigquery/tests/unit/test_client.py Outdated
Comment threadbigquery/tests/unit/test_client.py Outdated
* added new test to check 'client_options' as a dict
* deleted extra asserts
* chged object at 'test_ctor_w_client_options_object' to an alternative value
@IlyaFaerIlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 15, 2019
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 15, 2019
* fixed lint issues
@IlyaFaerIlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 16, 2019
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 16, 2019

@tswasttswast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for updating the tests, just one question.

Comment threadbigquery/setup.py
@busunkim96
busunkim96 requested a review from tswastAugust 27, 2019 17:59
@busunkim96
busunkim96 merged commit 814bd26 into googleapis:masterAug 27, 2019
@emar-kar
emar-kar deleted the add-client_options-to-bigquery-constructor branch August 28, 2019 07:59
HemangChothani pushed a commit to HemangChothani/google-cloud-python that referenced this pull request Aug 29, 2019
emar-kar added a commit to MaxxleLLC/google-cloud-python that referenced this pull request Sep 18, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigqueryIssues related to the BigQuery API.cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

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