Skip to content

Conversation

@vimota
Copy link
Contributor

@vimota vimota commented Dec 7, 2019

No description provided.

@vimota vimota requested review from mcollins42 and rosbo December 7, 2019 08:37
@vimota
Copy link
Contributor Author

vimota commented Dec 7, 2019

This works for all integrations except the automl TablesClient, they have a bug in their client initialization where if you provide a client_info= parameter it throws TypeError: type object got multiple values for keyword argument 'client_info'.

Cause of it is this line, where they get a client_info from kwargs, but then pass it as a parameter again, so it's passed in twice (as a named parameter and in the original kwargs):
https://github.com/googleapis/google-cloud-python/blob/9e5ca9a6e468d246e3aab5392ef0eeb4e802ec3d/automl/google/cloud/automl_v1beta1/tables/tables_client.py#L109

This should be easy to fix, I'll sent them an PR/Issue for it.

@vimota vimota marked this pull request as ready for review December 7, 2019 08:38
@rosbo
Copy link
Contributor

rosbo commented Dec 7, 2019

Note, the master build is failing and is likely the cause of this failure. I will try fixing it today. Once master is green, you will have to rebase from master to get all the latest changes and make sure the build is green. Thanks

@rosbo
Copy link
Contributor

rosbo commented Dec 7, 2019

I have fixed the master build. It should be good now.

Copy link
Contributor

@mcollins42 mcollins42 left a comment

Choose a reason for hiding this comment

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

LGTM!


# TODO(vimota): Remove the exclusion of TablesClient once
# the client has fixed the error:
# `multiple values for keyword argument 'client_info'``
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that an actual error that you see if this is set? I was hoping that Python would just ignore the kwargs value if a named value was already passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vimota
Copy link
Contributor Author

vimota commented Dec 9, 2019

I have fixed the master build. It should be good now.

Thanks, rebased and rerunning!

@vimota vimota merged commit f9b559a into master Dec 9, 2019
@vimota vimota deleted the useragent branch December 9, 2019 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants