-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Core: Add client options. #8265
Changes from 7 commits
8fc5a12
e56133a
6e74647
17caa54
be73bba
fc8e1a5
2f448b6
4447da0
8c6af56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
# Copyright 2019 Google LLC | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
"""Client options class. | ||
|
||
Client options provide a consistent interface for user options to be defined | ||
across clients. | ||
""" | ||
|
||
|
||
class ClientOptions(object): | ||
"""Client Options used to set options on clients. | ||
|
||
Args: | ||
api_endpoint (str): The desired API endpoint, e.g., compute.googleapis.com | ||
""" | ||
|
||
def __init__(self, api_endpoint=None): | ||
self.api_endpoint = api_endpoint | ||
|
||
|
||
def from_dict(options): | ||
"""Construct a client options object from a dictionary. | ||
|
||
Args: | ||
options (dict): A dictionary with client options. | ||
""" | ||
|
||
client_options = ClientOptions() | ||
|
||
for key, value in options.items(): | ||
if hasattr(client_options, key): | ||
setattr(client_options, key, value) | ||
else: | ||
raise ValueError("ClientOptions does not accept an option '" + key + "'") | ||
|
||
return client_options |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
# Copyright 2017 Google LLC | ||
busunkim96 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
import pytest | ||
|
||
from google.api_core import client_options | ||
|
||
|
||
def test_constructor(): | ||
options = client_options.ClientOptions(api_endpoint="foo.googleapis.com") | ||
|
||
assert options.api_endpoint == "foo.googleapis.com" | ||
|
||
|
||
def test_from_dict(): | ||
options = client_options.from_dict({"api_endpoint": "foo.googleapis.com"}) | ||
|
||
assert options.api_endpoint == "foo.googleapis.com" | ||
|
||
|
||
def test_from_dict_bad_argument(): | ||
with pytest.raises(ValueError): | ||
client_options.from_dict( | ||
{"api_endpoint": "foo.googleapis.com", "bad_arg": "1234"} | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
|
||
from google.oauth2 import service_account | ||
import google.api_core.gapic_v1.client_info | ||
import google.api_core.client_options | ||
import google.api_core.gapic_v1.config | ||
import google.api_core.gapic_v1.method | ||
import google.api_core.gapic_v1.routing_header | ||
|
@@ -112,6 +113,7 @@ def __init__( | |
credentials=None, | ||
client_config=None, | ||
client_info=None, | ||
client_options=None, | ||
): | ||
"""Constructor. | ||
|
||
|
@@ -142,6 +144,9 @@ def __init__( | |
API requests. If ``None``, then default info will be used. | ||
Generally, you only need to set this if you're developing | ||
your own client library. | ||
client_options (Union[dict, google.api_core.client_options.ClientOptions): | ||
Client options used to set user options on the client. API Endpoint | ||
should be set through client_options. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is going to be very confusing for users: we have three 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 the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 both There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: It is possible There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is a link to the extensible options in Python doc that should be accessible to all of [email protected]. It has more context about Client Options and what we're looking to add to it in the future. The goal of User agent is on the list of things we would like to be configurable with Luke's response as to why client_config was deprecated:
|
||
""" | ||
# Raise deprecation warnings for things we want to go away. | ||
if client_config is not None: | ||
|
@@ -160,6 +165,13 @@ def __init__( | |
stacklevel=2, | ||
) | ||
|
||
api_endpoint = self.SERVICE_ADDRESS | ||
if client_options: | ||
if type(client_options) == dict: | ||
client_options = google.api_core.client_options.from_dict(client_options) | ||
if client_options.api_endpoint: | ||
api_endpoint = client_options.api_endpoint | ||
|
||
# Instantiate the transport. | ||
# The transport is responsible for handling serialization and | ||
# deserialization and actually sending data to the service. | ||
|
@@ -168,6 +180,7 @@ def __init__( | |
self.transport = transport( | ||
credentials=credentials, | ||
default_class=device_manager_grpc_transport.DeviceManagerGrpcTransport, | ||
address=api_endpoint, | ||
) | ||
else: | ||
if credentials: | ||
|
@@ -178,7 +191,7 @@ def __init__( | |
self.transport = transport | ||
else: | ||
self.transport = device_manager_grpc_transport.DeviceManagerGrpcTransport( | ||
address=self.SERVICE_ADDRESS, channel=channel, credentials=credentials | ||
address=api_endpoint, channel=channel, credentials=credentials | ||
) | ||
|
||
if client_info is None: | ||
|
There was a problem hiding this comment.
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": "..."})