Movatterモバイル変換


[0]ホーム

URL:


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

Sign up
Appearance settings

Core: Add client options.#8265

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
busunkim96 merged 9 commits intogoogleapis:masterfrombusunkim96:client_options
Jun 24, 2019

Conversation

@busunkim96
Copy link
Contributor

@busunkim96busunkim96 commentedJun 7, 2019
edited
Loading

Thisjust adds API Endpoint (but more options will be coming, like credentials).

I manually edited the IoT client to show what changes need to be made to the generated clients. I will undo those changes before merging.

go/extensible-options-in-python <-- shared withyoshi-team@google.com
go/extensible-options-in-client

CC@shinfan

@googlebotgooglebot added the cla: yesThis human has signed the Contributor License Agreement. labelJun 7, 2019
your own client library.
client_options (google.api_core.client_options.ClientOptions):
Client options used to set user options on the client. API Endpoint
should be set through client_options.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to be very confusing for users: we havethree similarly-named arguments, one of which is deprecated, all of which exist to allow users to override bits of the default configuration for the client. Could weadd theapi_endpoint attribute togoogle.api_core.client_info.ClientInfo?

Choose a reason for hiding this comment

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

This client options class is designed to be the central place for client-level user configuration, not just for endpoints. We are planning to add credentials, pre-defined headers and other stuff.

I agree that right now it seems like we have 'client_config', 'client_info' and 'client_options' which all look the same. Is it possible that we merge these similar classes into one?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

We can look into rolling the others into client_options, since that is the "new" name that's going to be used across clients.

Copy link
Contributor

Choose a reason for hiding this comment

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

@busunkim96 I don't think you've addressed the confusion factor here: why do we want people to configure the client using bothclient_info andclient_options (leaving aside my never-answered question of whyclient_config is deprecated)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please correct me if any of this isn't correct, but I think:
client_config -> this is about gRPC timeout/retry configuration
client_info -> this should probably nameduser_agent_info or something more accurate. This is the data attached to requests to identify the particular client better.
client_options -> this new property bag, currently for endpoints. Eventually also additional things.

It is possibleuser_agent_info should also be underclient_options

tswast reacted with thumbs up emoji
Copy link
Contributor

Choose a reason for hiding this comment

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

Your description of the three arguments roles is correct. I'm arguing that three is two too many. Users should only need to thing about one thing to pass in to customize the behavior of the client.

@lukesneeringer Can you please comment on whyclient_config is deprecated? How else would users be expected to tweak e.g. gRPC timeouts for client-invoked methods?

Copy link
ContributorAuthor

@busunkim96busunkim96Jun 14, 2019
edited
Loading

Choose a reason for hiding this comment

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

Here is alink to the extensible options in Python doc that should be accessible to all ofyoshi-team@google.com. It has more context about Client Options and what we're looking to add to it in the future.

The goal ofclient_options is to have a more straight-forward way to configure clients. We want to provide a consistent interface to set options across the generated, handwritten, and Apiary clients. The experience will also be more predictable across languages.

User agent is on the list of things we would like to be configurable withclient_options. I think we want to move what is inclient_info (user agent) intoclient_options. In the near term users will be able to use both, with the user_agent inclient_options taking precedence. We will deprecateclient_info eventually.


Luke's response as to why client_config was deprecated:

The original rationale for deprecating that was that the client options were grpc specific and may not work well with other transports.

The idea was to use the transport itself as the replacement. Based on a request from Chris recently, however, we might need to rethink that, possibly by allowing folks to specify the arguments to the transport that are passed through (without having to instantiate the transport themselves).

Copy link

@shinfanshinfan left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. I left some comments.

your own client library.
client_options (google.api_core.client_options.ClientOptions):
Client options used to set user options on the client. API Endpoint
should be set through client_options.

Choose a reason for hiding this comment

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

This client options class is designed to be the central place for client-level user configuration, not just for endpoints. We are planning to add credentials, pre-defined headers and other stuff.

I agree that right now it seems like we have 'client_config', 'client_info' and 'client_options' which all look the same. Is it possible that we merge these similar classes into one?

@shinfan
Copy link

CC@wora

options (dict): A dictionary with client options.
"""

client_options=ClientOptions()
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

This would allow users to do something like:

client = pubsub_v1.PublisherClient(client_options={"api_endpoint": "...", "user_project": "..."})

@yoshi-automationyoshi-automation added the 🚨This issue needs some love. labelJun 15, 2019
@busunkim96busunkim96 requested a review fromtseaverJune 19, 2019 18:20
Copy link
Contributor

@tswasttswast left a comment

Choose a reason for hiding this comment

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

I'm satisfied with this direction, given the design doc has been approved and we now have a plan for all these "options, info, config" objects.

@shinfan
Copy link

Thanks for reviewing this Tim.

@busunkim96busunkim96 merged commite80709c intogoogleapis:masterJun 24, 2019
@busunkim96busunkim96 deleted the client_options branchJune 24, 2019 17:18
parthea pushed a commit that referenced this pull requestNov 24, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@tswasttswasttswast approved these changes

@crwilcoxcrwilcoxAwaiting requested review from crwilcox

@tseavertseaverAwaiting requested review from tseaver

+1 more reviewer

@shinfanshinfanshinfan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yesThis human has signed the Contributor License Agreement.🚨This issue needs some love.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@busunkim96@shinfan@tseaver@tswast@crwilcox@googlebot@yoshi-automation

[8]ページ先頭

©2009-2025 Movatter.jp