FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Allows Explicitly Enabling/Disabling GAX for Logging/Pubsub by waprin · Pull Request #2553 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

Allows Explicitly Enabling/Disabling GAX for Logging/Pubsub - #2553

Merged
dhermes merged 6 commits into
googleapis:masterfrom
waprin:logging_gax_optional
Oct 21, 2016
ConversationCommits (6)ChecksFiles changed
Merged

Allows Explicitly Enabling/Disabling GAX for Logging/Pubsub#2553
dhermes merged 6 commits into
googleapis:masterfrom
waprin:logging_gax_optional

Conversation

waprin commented Oct 17, 2016

Copy link
Copy Markdown
Contributor

Currently gax can’t log nested structs, so this is a workaround to allow forced HTTP logging.

Workaround to resolve #2521 until #2552 is resolved.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 17, 2016
dhermes reviewed Oct 17, 2016
View reviewed changes
Comment thread logging/google/cloud/logging/client.py Outdated
def __init__(self, project=None, credentials=None,
http=None, use_gax=True):
super(Client, self).__init__(project, credentials, http)
self.use_gax = use_gax

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dhermes commented Oct 17, 2016

Copy link
Copy Markdown
Contributor

Not sure this will cause coverage to go below 100% because the branches are all implicit in one-line or statements. But I can see the branches. So pretty please write some unit tests?

Also update the logging Client docstring with the new argument (pretty sure Pylint will get mad about that).

daspecster commented Oct 17, 2016

Copy link
Copy Markdown
Contributor

What does this add that the DISABLE_GRPC envar doesn't do? Could the workaround just be setting that to True?

dhermes commented Oct 17, 2016

Copy link
Copy Markdown
Contributor

@daspecster It's a manual override, since the gRPC client is b0rken (see the issue that @waprin linked to).

waprin commented Oct 17, 2016

Copy link
Copy Markdown
Contributor Author

@daspecster Yes but error reporting currently uses the logging client, it doesn't seem right for one module to be forcing a global environment variable.

daspecster commented Oct 17, 2016

Copy link
Copy Markdown
Contributor

Yeah I looked at the issues. If this gets added, then we may want to set a reminder/issue to take it out once the bug in gRPC is resolved right?

Enable forced HTTP for Error Reporting
bda9705 
Currently gax can’t log nested structs, so this
is a workaround to allow forced HTTP logging.
waprin force-pushed the logging_gax_optional branch from f45bfe2 to c0cd817 Compare October 17, 2016 21:43

waprin commented Oct 17, 2016

Copy link
Copy Markdown
Contributor Author

@dhermes added unit test and coverage passes.

daspecster mentioned this pull request Oct 17, 2016
Closed
dhermes reviewed Oct 17, 2016
View reviewed changes
Comment thread logging/unit_tests/test_client.py


class TestClient(unittest.TestCase):

This comment was marked as spam.

Sign in to view

Comment thread logging/unit_tests/test_client.py
return wrapped

class _GaxLoggingAPI(object):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread logging/unit_tests/test_client.py


class _DummyLoggingAPI(object):

This comment was marked as spam.

Sign in to view

waprin force-pushed the logging_gax_optional branch from c0cd817 to d417c1d Compare October 17, 2016 23:59
waprin force-pushed the logging_gax_optional branch from d417c1d to fceaacf Compare October 18, 2016 00:00

daspecster commented Oct 19, 2016

Copy link
Copy Markdown
Contributor

@waprin looks like we have a fix to support that structure.

dhermes commented Oct 19, 2016

Copy link
Copy Markdown
Contributor

@waprin I think a few things should happen:

  1. Close this PR
  2. We can have a new PR which allows use_gax as a constructor argument in all GAX / GAPIC / gRPC supported APIs (or re-purpose this PR to do the same)
  3. We fix error reporting / logging by fixing our b0rken code

waprin commented Oct 19, 2016

Copy link
Copy Markdown
Contributor Author

@dhermes SGTM, will just re-purpose this PR

waprin changed the title Enable forced HTTP for Error Reporting Add JSONOrGaxClient That Allows Explicitly Enabling/Disabling GAX Oct 20, 2016
waprin force-pushed the logging_gax_optional branch from 3ffc5cd to b834d13 Compare October 20, 2016 21:24
waprin changed the title Add JSONOrGaxClient That Allows Explicitly Enabling/Disabling GAX Allows Explicitly Enabling/Disabling GAX for Logging/Pubsub Oct 20, 2016

waprin commented Oct 20, 2016

Copy link
Copy Markdown
Contributor Author

@dhermes added pub/sub.

waprin force-pushed the logging_gax_optional branch from b834d13 to 950afd1 Compare October 20, 2016 22:30
waprin force-pushed the logging_gax_optional branch from 950afd1 to 088e8b4 Compare October 20, 2016 22:31
dhermes reviewed Oct 21, 2016
View reviewed changes
Comment thread error_reporting/google/cloud/error_reporting/client.py Outdated
version=None):
self.logging_client = google.cloud.logging.client.Client(
project, credentials, http)
project=project, credentials=credentials, http=http, use_gax=False)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread logging/google/cloud/logging/client.py Outdated

:type use_gax: bool or :class:`NoneType`
:param use_gax: An optional parameter that explicitly specifies whether
to use the gRPC transport (gax) or HTTP

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread logging/unit_tests/test_client.py Outdated

with _Monkey(MUT, _USE_GAX=False):
client = self._makeOne(self.PROJECT, credentials=_Credentials())
conn = client.connection = object()

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread logging/unit_tests/test_client.py Outdated

creds = _Credentials()
with _Monkey(MUT,
_USE_GAX=True):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread logging/unit_tests/test_client.py
def test_no_gax_ctor(self):
from google.cloud.logging.connection import _LoggingAPI
from google.cloud.logging import client as MUT
from google.cloud._testing import _Monkey

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread logging/google/cloud/logging/client.py Outdated
``http`` object is created that is bound to the
``credentials`` for the current object.

:type use_gax: bool or :class:`NoneType`

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread logging/google/cloud/logging/client.py Outdated
``credentials`` for the current object.

:type use_gax: bool or :class:`NoneType`
:param use_gax: An optional parameter that explicitly specifies whether

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/unit_tests/test_client.py
conn = client.connection = object()

with _Monkey(MUT, _USE_GAX=False):
client = self._makeOne(project=self.PROJECT, credentials=creds)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Comment thread pubsub/unit_tests/test_client.py
conn = client.connection = object()

with _Monkey(MUT, _USE_GAX=False):
client = self._makeOne(project=self.PROJECT, credentials=creds)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dhermes approved these changes Oct 21, 2016
View reviewed changes

dhermes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM (merge squash in the UI once Travis is green?)

waprin commented Oct 21, 2016

Copy link
Copy Markdown
Contributor Author

@dhermes yes please squash merge at your convienence

dhermes merged commit 1fab756 into googleapis:master Oct 21, 2016
dhermes mentioned this pull request Nov 14, 2016
Merged
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
Merge pull request googleapis#2553 from waprin/logging_gax_optional
344cd8e 
Allows Explicitly Enabling/Disabling GAX for Logging/Pubsub
parthea pushed a commit that referenced this pull request Nov 24, 2025
Merge pull request #2553 from waprin/logging_gax_optional
7ee974b 
Allows Explicitly Enabling/Disabling GAX for Logging/Pubsub
parthea pushed a commit that referenced this pull request Mar 2, 2026
Merge pull request #2553 from waprin/logging_gax_optional
c5b0539 
Allows Explicitly Enabling/Disabling GAX for Logging/Pubsub
ohmayr added a commit that referenced this pull request Mar 13, 2026
chore: use import absolute.path as module syntax for pb2 imports (#2553)
085bd41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
dhermes dhermes approved these changes
Reviewers whose approvals may not affect merge requirements

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.

Stackdriver ErrorReporter Unexpected Type

4 participants

Back | FazBrowse Home | New Git URL