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

fix: client version missing from the user agent header #275

Merged
merged 2 commits into from
Jan 28, 2021
Merged

fix: client version missing from the user agent header #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
13c7105
fix: missing client version in user agent header
plamut Jan 15, 2021
644a83e
Blacken
plamut Jan 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion google/pubsub_v1/services/publisher/async_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,9 @@ async def test_iam_permissions(

try:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=pkg_resources.get_distribution("google-pubsub",).version,
client_library_version=pkg_resources.get_distribution(
"google-cloud-pubsub",
).version,
)
except pkg_resources.DistributionNotFound:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo()
Expand Down
4 changes: 3 additions & 1 deletion google/pubsub_v1/services/publisher/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1315,7 +1315,9 @@ def test_iam_permissions(

try:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=pkg_resources.get_distribution("google-pubsub",).version,
client_library_version=pkg_resources.get_distribution(
"google-cloud-pubsub",
).version,
)
except pkg_resources.DistributionNotFound:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo()
Expand Down
4 changes: 3 additions & 1 deletion google/pubsub_v1/services/publisher/transports/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@

try:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=pkg_resources.get_distribution("google-pubsub",).version,
client_library_version=pkg_resources.get_distribution(
"google-cloud-pubsub",
).version,
)
except pkg_resources.DistributionNotFound:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo()
Expand Down
4 changes: 3 additions & 1 deletion google/pubsub_v1/services/schema_service/async_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,9 @@ async def test_iam_permissions(

try:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=pkg_resources.get_distribution("google-pubsub",).version,
client_library_version=pkg_resources.get_distribution(
"google-cloud-pubsub",
).version,
)
except pkg_resources.DistributionNotFound:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo()
Expand Down
4 changes: 3 additions & 1 deletion google/pubsub_v1/services/schema_service/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,9 @@ def test_iam_permissions(

try:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=pkg_resources.get_distribution("google-pubsub",).version,
client_library_version=pkg_resources.get_distribution(
"google-cloud-pubsub",
).version,
)
except pkg_resources.DistributionNotFound:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo()
Expand Down
4 changes: 3 additions & 1 deletion google/pubsub_v1/services/schema_service/transports/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@

try:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=pkg_resources.get_distribution("google-pubsub",).version,
client_library_version=pkg_resources.get_distribution(
"google-cloud-pubsub",
).version,
)
except pkg_resources.DistributionNotFound:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo()
Expand Down
4 changes: 3 additions & 1 deletion google/pubsub_v1/services/subscriber/async_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1874,7 +1874,9 @@ async def test_iam_permissions(

try:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=pkg_resources.get_distribution("google-pubsub",).version,
client_library_version=pkg_resources.get_distribution(
"google-cloud-pubsub",
).version,
)
except pkg_resources.DistributionNotFound:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo()
Expand Down
4 changes: 3 additions & 1 deletion google/pubsub_v1/services/subscriber/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2014,7 +2014,9 @@ def test_iam_permissions(

try:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=pkg_resources.get_distribution("google-pubsub",).version,
client_library_version=pkg_resources.get_distribution(
"google-cloud-pubsub",
).version,
)
except pkg_resources.DistributionNotFound:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo()
Expand Down
4 changes: 3 additions & 1 deletion google/pubsub_v1/services/subscriber/transports/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@

try:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo(
gapic_version=pkg_resources.get_distribution("google-pubsub",).version,
client_library_version=pkg_resources.get_distribution(
"google-cloud-pubsub",
).version,
)
except pkg_resources.DistributionNotFound:
DEFAULT_CLIENT_INFO = gapic_v1.client_info.ClientInfo()
Expand Down
14 changes: 7 additions & 7 deletions synth.metadata
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,37 +3,37 @@
{
"git": {
"name": ".",
"remote": "[email protected]:googleapis/python-pubsub",
"sha": "aa45340d999f845c67396e8740e96f8a8caafd16"
"remote": "[email protected]:plamut/python-pubsub.git",
"sha": "a4eab77decdd7ea0d421b56a784e8a673a5595ec"
}
},
{
"git": {
"name": "googleapis",
"remote": "https://github.com/googleapis/googleapis.git",
"sha": "57fc4a8a94a5bd015a83fb0f0a1707f62254b2cd",
"internalRef": "348813319"
"sha": "61ab0348bd228c942898aee291d677f0afdb888c",
"internalRef": "352069361"
}
},
{
"git": {
"name": "synthtool",
"remote": "https://github.com/googleapis/synthtool.git",
"sha": "6b026e1443948dcfc0b9e3289c85e940eb70f694"
"sha": "56ddc68f36b32341e9f22c2c59b4ce6aa3ba635f"
}
},
{
"git": {
"name": "synthtool",
"remote": "https://github.com/googleapis/synthtool.git",
"sha": "6b026e1443948dcfc0b9e3289c85e940eb70f694"
"sha": "56ddc68f36b32341e9f22c2c59b4ce6aa3ba635f"
}
},
{
"git": {
"name": "synthtool",
"remote": "https://github.com/googleapis/synthtool.git",
"sha": "6b026e1443948dcfc0b9e3289c85e940eb70f694"
"sha": "56ddc68f36b32341e9f22c2c59b4ce6aa3ba635f"
}
}
],
Expand Down
17 changes: 17 additions & 0 deletions synth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,23 @@
\g<0>""",
)

# Make sure that client library version is present in user agent header.
s.replace(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something we can update in the Bazel config to avoid manual overrides? https://github.com/googleapis/googleapis/blob/master/google/pubsub/v1/BUILD.bazel

Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite familiar with BUILD.bazel, but it looks like pubsub should belong in https://github.com/googleapis/googleapis/tree/master/google/cloud just as pubsublite?

[
"google/pubsub_v1/services/publisher/async_client.py",
"google/pubsub_v1/services/publisher/client.py",
"google/pubsub_v1/services/publisher/transports/base.py",
"google/pubsub_v1/services/schema_service/async_client.py",
"google/pubsub_v1/services/schema_service/client.py",
"google/pubsub_v1/services/schema_service/transports/base.py",
"google/pubsub_v1/services/subscriber/async_client.py",
"google/pubsub_v1/services/subscriber/client.py",
"google/pubsub_v1/services/subscriber/transports/base.py",
],
r"""gapic_version=(pkg_resources\.get_distribution\(\s+)['"]google-pubsub['"]""",
"client_library_version=\g<1>'google-cloud-pubsub'",
)

# Docstrings of *_iam_policy() methods are formatted poorly and must be fixed
# in order to avoid docstring format warnings in docs.
s.replace("google/pubsub_v1/services/*er/client.py", r"(\s+)Args:", "\n\g<1>Args:")
Expand Down
29 changes: 25 additions & 4 deletions tests/unit/pubsub_v1/publisher/test_publisher_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

from google.api_core import gapic_v1
from google.api_core import retry as retries
from google.api_core.gapic_v1.client_info import METRICS_METADATA_KEY
from google.cloud.pubsub_v1 import publisher
from google.cloud.pubsub_v1 import types

Expand Down Expand Up @@ -62,6 +63,26 @@ def test_init():
assert client.batch_settings.max_messages == 100


def test_init_default_client_info():
creds = mock.Mock(spec=credentials.Credentials)
client = publisher.Client(credentials=creds)

installed_version = publisher.client.__version__
expected_client_info = f"gccl/{installed_version}"

for wrapped_method in client.api.transport._wrapped_methods.values():
user_agent = next(
(
header_value
for header, header_value in wrapped_method._metadata
if header == METRICS_METADATA_KEY
),
None,
)
assert user_agent is not None
assert expected_client_info in user_agent


def test_init_w_custom_transport():
transport = PublisherGrpcTransport()
client = publisher.Client(transport=transport)
Expand All @@ -86,7 +107,7 @@ def test_init_w_api_endpoint():


def test_init_w_unicode_api_endpoint():
client_options = {"api_endpoint": u"testendpoint.google.com"}
client_options = {"api_endpoint": "testendpoint.google.com"}
client = publisher.Client(client_options=client_options)

assert isinstance(client.api, publisher_client.PublisherClient)
Expand Down Expand Up @@ -219,7 +240,7 @@ def test_publish_data_not_bytestring_error():
client = publisher.Client(credentials=creds)
topic = "topic/path"
with pytest.raises(TypeError):
client.publish(topic, u"This is a text string.")
client.publish(topic, "This is a text string.")
with pytest.raises(TypeError):
client.publish(topic, 42)

Expand Down Expand Up @@ -300,7 +321,7 @@ def test_publish_attrs_bytestring():

# The attributes should have been sent as text.
batch.publish.assert_called_once_with(
gapic_types.PubsubMessage(data=b"foo", attributes={"bar": u"baz"})
gapic_types.PubsubMessage(data=b"foo", attributes={"bar": "baz"})
)


Expand Down Expand Up @@ -339,7 +360,7 @@ def test_publish_new_batch_needed():
commit_when_full=True,
commit_retry=gapic_v1.method.DEFAULT,
)
message_pb = gapic_types.PubsubMessage(data=b"foo", attributes={"bar": u"baz"})
message_pb = gapic_types.PubsubMessage(data=b"foo", attributes={"bar": "baz"})
batch1.publish.assert_called_once_with(message_pb)
batch2.publish.assert_called_once_with(message_pb)

Expand Down
23 changes: 22 additions & 1 deletion tests/unit/pubsub_v1/subscriber/test_subscriber_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import grpc
import mock

from google.api_core.gapic_v1.client_info import METRICS_METADATA_KEY
from google.cloud.pubsub_v1 import subscriber
from google.cloud.pubsub_v1 import types
from google.cloud.pubsub_v1.subscriber import futures
Expand All @@ -29,6 +30,26 @@ def test_init():
assert isinstance(client.api, subscriber_client.SubscriberClient)


def test_init_default_client_info():
creds = mock.Mock(spec=credentials.Credentials)
client = subscriber.Client(credentials=creds)

installed_version = subscriber.client.__version__
expected_client_info = f"gccl/{installed_version}"

for wrapped_method in client.api.transport._wrapped_methods.values():
user_agent = next(
(
header_value
for header, header_value in wrapped_method._metadata
if header == METRICS_METADATA_KEY
),
None,
)
assert user_agent is not None
assert expected_client_info in user_agent


def test_init_w_custom_transport():
transport = SubscriberGrpcTransport()
client = subscriber.Client(transport=transport)
Expand All @@ -47,7 +68,7 @@ def test_init_w_api_endpoint():


def test_init_w_unicode_api_endpoint():
client_options = {"api_endpoint": u"testendpoint.google.com"}
client_options = {"api_endpoint": "testendpoint.google.com"}
client = subscriber.Client(client_options=client_options)

assert isinstance(client.api, subscriber_client.SubscriberClient)
Expand Down