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

Various hygiene changes to Pub / Sub subscriber. #4494

Merged
merged 1 commit into from
Nov 29, 2017

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Nov 29, 2017

  • Using %-formatting in all logging.log() calls (e.g. info()). I am not opposed to using .format() but logging "prefers" %-formatting (and I wanted to be consistent because hobgoblins).
  • Adding non-public globals for helper thread names, this was especially needed because I accidentally broke this in Renaming "Consumer helper" threads in Pub/Sub. #4476 when I changed callback requests worker to CallbackRequestsWorker in one place, not two.
  • Adding docstring to QueueCallbackThread that explains what it does (I'll refactor this class in a follow-up)
  • Adding a logging statement when a QueueCallbackThread exits
  • Changing indents / using much more vertical space in calls to request_queue.put() in subscriber.message.Message. (I came across these when trying to understand how QueueCallbackThread interacts with Policy.on_callback_request)
  • Logging an exit statement when maintain_leases exits
  • Changing GPRC to gRPC in a docstring
  • Moving "Creating callback requests thread (not starting)." until right before the resource is created
  • Changing "Spawning" to "Starting" in a log message to match others

NOTE: Many of these can be construed as "nit picks" but I wanted to get them out of my staged changed while working on #4463.

@dhermes dhermes added the api: pubsub Issues related to the Pub/Sub API. label Nov 29, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 29, 2017
@dhermes dhermes added type: cleanup An internal cleanup or hygiene concern. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 29, 2017
- Using `%`-formatting in all `logging.log()` calls (e.g. `info()`).
  I am not opposed to using `.format()` but `logging` "prefers"
  `%`-formatting (and I wanted to be consistent because hobgoblins).
- Adding non-public globals for helper thread names, this was
  especially needed because I accidentally broke this in googleapis#4476 when
  I changed `callback requests worker` to `CallbackRequestsWorker`
  in one place, not two.
- Adding docstring to `QueueCallbackThread` that explains what it
  does (I'll refactor this class in a follow-up)
- Adding a logging statement when a `QueueCallbackThread` exits
- Changing indents / using much more vertical space in calls to
  `request_queue.put()` in `subscriber.message.Message`. (I came across
  these when trying to understand how `QueueCallbackThread` interacts
  with `Policy.on_callback_request`)
- Changing `GPRC` to `gRPC` in a docstring
- Moving "Creating callback requests thread (not starting)." until
  right before the resource is created
- Changing "Spawning" to "Starting" in a log message to match others
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 29, 2017
@dhermes dhermes merged commit 6518d4f into googleapis:master Nov 29, 2017
@dhermes dhermes deleted the pubsub-style branch November 29, 2017 23:25
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