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.

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

fix(automl): fix TypeError when passing a client_info to automl TablesClient #9949

Merged
merged 3 commits into from
Dec 17, 2019
Merged

fix(automl): fix TypeError when passing a client_info to automl TablesClient #9949

merged 3 commits into from
Dec 17, 2019

Conversation

vimota
Copy link
Contributor

@vimota vimota commented Dec 9, 2019

Ensures the ClientInfo object isn't passed in twice to the underlying clients (AutoMLClient and PredictionServiceClient) from the Table Client.

Fixes #9948 🦕

@vimota vimota requested a review from busunkim96 as a code owner December 9, 2019 19:45
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 9, 2019
@vimota
Copy link
Contributor Author

vimota commented Dec 9, 2019

Cc'ing @helinwang as I was told you are the new maintainer of the library.

@busunkim96 busunkim96 requested a review from helinwang December 12, 2019 17:56
helinwang
helinwang reviewed Dec 12, 2019
View reviewed changes
automl/google/cloud/automl_v1beta1/tables/tables_client.py
@@ -104,17 +104,18 @@ def __init__(
else:
client_info_.user_agent = user_agent
client_info_.gapic_version = version
Copy link
Contributor

@helinwang helinwang Dec 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the PR!
I think it's good to encourage being explicit with the argument. E.g., prefer explicit argument than using kwargs.
Instead of removing the client_info argument, could you change the solution to removing "client_info" from kwargs after this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

@helinwang
Copy link
Contributor

The check is failing on Commit message did not follow Conventional Commits, could you make change to the commit message according to the rule as well? Thanks!

@vimota vimota changed the title Fix TypeError when passing a client_info to automl TablesClient fix(automl): fix TypeError when passing a client_info to automl TablesClient Dec 12, 2019
@vimota
Copy link
Contributor Author

vimota commented Dec 12, 2019

The check is failing on Commit message did not follow Conventional Commits, could you make change to the commit message according to the rule as well? Thanks!

Done!

helinwang
helinwang approved these changes Dec 12, 2019
View reviewed changes
Copy link
Contributor

@helinwang helinwang left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, thanks!

@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 13, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 13, 2019
@vimota
Copy link
Contributor Author

vimota commented Dec 17, 2019

Fix the formatting that failed the test. (running blacken with python version 3.6 failed with No module named 'distutils.spawn' so I had to temporarily run it with 3.7 to fix the lint errors).

@vimota
Copy link
Contributor Author

vimota commented Dec 17, 2019

@busunkim96 Is it possible to rerun the presubmits? Thanks!

@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 17, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 17, 2019
@busunkim96
Copy link
Contributor

@vimota Done!. @helinwang please merge when you are ready to.

@helinwang
Copy link
Contributor

Thanks all!

@helinwang helinwang merged commit 8bb4068 into googleapis:master Dec 17, 2019
This was referenced Jan 29, 2020
Merged
Merged
Closed
Closed
parthea pushed a commit that referenced this pull request Oct 21, 2023
@vimota @helinwang
fix(automl): fix TypeError when passing a client_info to automl Table…
bf706fd 
…sClient (#9949)

* fix(automl): fix TypeError when passing a client_info to automl TablesClient

The solution is to delete client_info from kwargs instead and pass named parameter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@helinwang helinwang helinwang approved these changes

@busunkim96 busunkim96 Awaiting requested review from busunkim96

Assignees
No one assigned
Labels
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.

AutoML Tables: Passing client_info produces a a TypeError
5 participants
@vimota @helinwang @busunkim96 @googlebot @yoshi-kokoro