Skip to content

Conversation

@mcollins42
Copy link
Contributor

Changes:

  • GcsClient.__init__ takes an optional project parameter and passes it to the storage.Client constructor.
  • TablesClient stores the credentials passed to the constructor and passes the credential when creating the GcsClient
  • Ensure that project, region and credentials fall back to the values passed to the TablesClient constructor if no value is passed as an override on public methods

I had a little trouble testing that parameters were passed to the constructors for GcsClient and storage.Client. I tried various combinations of mock and patch, but even MagicMock doesn't support mocking __init__(). Any suggestions on better approaches would be appreciated.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 25, 2019
@mcollins42 mcollins42 changed the title Add support for passing a project to GcsClient and for TablesClient passing the default project and credentials fix(automl): Add support for passing a project to GcsClient and for TablesClient passing the default project and credentials Sep 25, 2019
Copy link
Contributor

@lwander lwander left a comment

Choose a reason for hiding this comment

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

Nice! LGTM

@lwander
Copy link
Contributor

lwander commented Sep 25, 2019

Looks like you need to change the commit message (rather than the PR title): https://github.com/googleapis/google-cloud-python/pull/9299/checks?check_run_id=236270561

git commit --amend -m "fix(automl): Add support for passing a project to GcsClient and for TablesClient passing the default project and credentials" should do the trick

@mcollins42 mcollins42 changed the title fix(automl): Add support for passing a project to GcsClient and for TablesClient passing the default project and credentials Add support for passing a project to GcsClient and for TablesClient passing the default project and credentials Sep 25, 2019
@mcollins42
Copy link
Contributor Author

Looks like you need to change the commit message (rather than the PR title): https://github.com/googleapis/google-cloud-python/pull/9299/checks?check_run_id=236270561

I've updated the commit messages, but the checks haven't run again. Do I need to make a trivial commit or is there another way to force the checks to run again?

@mcollins42 mcollins42 changed the title Add support for passing a project to GcsClient and for TablesClient passing the default project and credentials fix(automl): add support for passing a project to GcsClient and for TablesClient passing the default project and credentials Sep 25, 2019
@tseaver tseaver added api: automl Issues related to the AutoML API. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Sep 26, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 26, 2019
@tseaver tseaver changed the title fix(automl): add support for passing a project to GcsClient and for TablesClient passing the default project and credentials enhancement(automl): support for passing project to 'GcsClient' Sep 26, 2019
Copy link
Contributor

@TrucHLe TrucHLe left a comment

Choose a reason for hiding this comment

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

LG! Thank you ☺️

@mcollins42
Copy link
Contributor Author

@tseaver Should I also update the type on the commit messages from fix to enhancement?

self.client = storage.Client()

self.bucket_name = bucket_name
self.project = project
Copy link
Contributor

Choose a reason for hiding this comment

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

This attribute is unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed and updated the docstring. Thanks!

@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 27, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 27, 2019
@busunkim96 busunkim96 requested a review from sirtorry September 27, 2019 20:25
@busunkim96 busunkim96 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
@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 1, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 1, 2019
@tseaver tseaver merged commit 619bcc9 into googleapis:master Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants