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.

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

Pub/Sub logging hygiene. #4471

Merged
merged 2 commits into from
Nov 28, 2017
Merged

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Nov 28, 2017

Uses newline character when logging protobufs that may end up having newlines in them. These are harder to grok in the log output since it "blends" the protobuf and the description. For example.

Received response: received_messages {
  ack_id: "Pn41MEV..."
  message {
    data: ...

vs.

Received response:
received_messages {
  ack_id: "Pn41MEV..."
  message {
    data: ...

Also

  • replaced a lambda with a simple function
  • added docstrings to two default callbacks used by the policy.thread.Policy implementation.
  • updated grammatical error in a comment
  • replaced a bare if future with if future is not None

Context: I have discovered each of these "issues" while dealing with #4463 and #4234.

@dhermes dhermes added api: pubsub Issues related to the Pub/Sub API. type: cleanup An internal cleanup or hygiene concern. labels Nov 28, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 28, 2017
@@ -167,7 +191,7 @@ def on_response(self, response):
For each message, schedule a callback with the executor.
"""
for msg in response.received_messages:
_LOGGER.debug('New message received from Pub/Sub: %r', msg)
_LOGGER.debug('New message received from Pub/Sub:\n%r', msg)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Uses newline character when logging protobufs that may end up having
newlines in them. These are harder to grok in the log output since
it "blends" the protobuf and the description. For example.

    Received response: received_messages {
      ack_id: "Pn41MEV..."
      message {
        data: ...

vs.

    Received response:
    received_messages {
      ack_id: "Pn41MEV..."
      message {
        data: ...

Also

- replaced a lambda with a simple function
- added docstrings to two default callbacks used by the
  `policy.thread.Policy` implementation.
- updated grammatical error in a comment
- replaced a bare `if future` with `if future is not None`
@dhermes
Copy link
Contributor Author

dhermes commented Nov 28, 2017

Changed 3 instances of %s to %r (channeling Luke). Will merge without waiting for CI to finish as this change doesn't impact any of the tests (I checked locally).

@dhermes dhermes merged commit 56fc884 into googleapis:master Nov 28, 2017
@dhermes dhermes deleted the pubsub-logging-hygiene branch November 28, 2017 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants