Skip to content

Add methods to connection mapping API requests #1700

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

Jump to bottom
Merged
tseaver merged 25 commits into from
Apr 22, 2016
Merged

Add methods to connection mapping API requests #1700

tseaver merged 25 commits into from
Apr 22, 2016

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented Apr 7, 2016

Encapsulate JSON/REST-specific knowledge inside the connection, allowing later use of a gRPC-based connection.

Toward #851 and #1696.

@tseaver tseaver added the api: pubsub Issues related to the Pub/Sub API. label Apr 7, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 7, 2016
@tseaver
Copy link
Contributor Author

tseaver commented Apr 7, 2016

@dhermes this may be easiest to review commit-by-commit: I tried hard to keep them logical and clean (passing coverage and lint on my machine at each commit).

If needed, I can chop this up into smaller pieces.

@dhermes
Copy link
Contributor

dhermes commented Apr 8, 2016

@tseaver Reviewing now. Sorry for the delay.

dhermes
dhermes reviewed Apr 8, 2016
View reviewed changes
gcloud/pubsub/client.py
resp = self.connection.api_request(method='GET', path=path,
query_params=params)
conn = self.connection
resources, next_token = conn.list_topics(

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

@dhermes
Copy link
Contributor

dhermes commented Apr 8, 2016

OK I finished my review. Overall pretty clean given the magnitude of the changes.

tseaver added 4 commits April 20, 2016 11:30
@tseaver
Move guts of 'Client.list_topics' to 'Connection.list_topics'.
d276229 
Client method now just rewraps resources returned by connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Client.list_subscriptions' to 'Connection.list_subscrip…
c65cf74 
…tions'.

Client method now just rewraps resources returned by connection.

Toward #851 and #1696.
@tseaver
Normalize, reduce boilerplate.
356f925
@tseaver
Move guts of 'Topic.create' to 'Connection.topic_create'.
3e53eeb 
Topic method now just delegates to the connection.

Toward #851 and #1696.
tseaver added 16 commits April 20, 2016 11:30
@tseaver
Move guts of 'Topic.set_iam_policy' to 'Connection.set_iam_policy'.
154a624 
Topic method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Topic.check_iam_permissions' to 'Connection.test_iam_pe…
086c321 
…rmissions'.

Topic method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.create' to 'Connection.subscription_create'.
cb8dd9d 
Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.exists' to 'Connection.subscription_get'.
d5e8660 
Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.reload' to 'Connection.subscription_get'.
a2ddf7e 
Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.delete' to 'Connection.subscription_delete'.
c40344f 
Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.modify_push_config' to 'Connection.subscri…
b82b764 
…ption_modify_push_config'.

Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.pull' to 'Connection.subscription_pull'.
5edb73e 
Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.modify_ack_deadline' to 'Connection.subscr…
d19da2e 
…iption_modify_ack_deadline'.

Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.get_iam_policy' to 'Connection.get_iam_pol…
94d81fa 
…icy'.

Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.set_iam_policy' to 'Connection.set_iam_pol…
ac2670a 
…icy'.

Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Move guts of 'Subscription.check_iam_permissions' to 'Connection.test…
3707e6d 
…_iam_permissions'.

Subscription method now just delegates to the connection.

Toward #851 and #1696.
@tseaver
Fix py34 JSON marshalling of message payload bytes.
1135b6f
@tseaver
Break out definition of 'path' for clarity.
f7ea446 
Addresses:
#1700 (comment)
@tseaver
Add ':rtype:' and ':returns:' for Connection methods.
9adf05b 
Addresses:
#1700 (comment)
and ff.
@tseaver
Make 'Connection.topic_publish' return a bare list of message IDs.
a8ee698
@tseaver
Copy link
Contributor Author

tseaver commented Apr 20, 2016

@dhermes PTAL: 0e337fa through 19b4f37 break the API-specific methods out of Connection into stub-like API helper classes, instantiated as properties of the Client. I think adding the gRPC versions will be a matter of creating parallel classes for them, and tweaking how the client chooses which to instantiate.

@tseaver tseaver mentioned this pull request Apr 20, 2016
Closed
@dhermes
Copy link
Contributor

dhermes commented Apr 22, 2016

@tseaver can we peel those off onto a new PR. This one was already quite heavy.

@tseaver
Copy link
Contributor Author

tseaver commented Apr 22, 2016

@dhermes Are OK to merge this one as of a8ee698?

@tseaver tseaver mentioned this pull request Apr 22, 2016
Merged
@tseaver
Copy link
Contributor Author

tseaver commented Apr 22, 2016

I just stripped off the"API helper" commits, and submitted them in #1736.

@dhermes
Copy link
Contributor

dhermes commented Apr 22, 2016

I'm pretty sure the answer is yes, double checking right now.

@dhermes
Copy link
Contributor

dhermes commented Apr 22, 2016

OK LGTM to merge

@tseaver tseaver merged commit 18c92fe into googleapis:master Apr 22, 2016
@tseaver tseaver deleted the 851-1696-pubsub-connection_maps_api branch April 22, 2016 16:13
@dhermes dhermes mentioned this pull request Apr 22, 2016
Merged
@tseaver tseaver mentioned this pull request May 24, 2016
Merged
parthea pushed a commit that referenced this pull request Nov 26, 2025
@ohmayr @gcf-owl-bot
feat: add request response logging to auth (#1678)
7fa3e06 
* feat: add functionality to hash data (#1677)

* feat: add functionality to hash data

* change sensitive fields to private

* update to sha512

* update docstring

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: add request-response log helpers (#1685)

* chore: add request-response log helpers

* fix presubmit

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* feat: opt-in logging support for request / response (#1686)

* feat: opt-in logging support for request/response

* add pragma no cover

* add test coverage for request/response

* add code coverage

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: remove logging for async requests (#1698)

* chore: remove logging for async requests

* change Dict to Mapping

* fix mypy and lint issues

* address PR feedback

* link issue

* feat: parse request/response for logging (#1696)

* feat: parse request/response for logging

* add test case for list

* address PR comments

* address PR feedback

* fix typo

* add test coverage

* add code coverage

* feat: hash sensitive info in logs (#1700)

* feat: hash sensitive info in logs

* make helper private

* add code coverage

* address PR feedback

* fix mypy type issue

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* feat: add support for async response log (#1733)

* feat: add support for async response log

* fix whitespace

* add await

* add code coverage

* fix lint

* fix lint issues

* address PR feedback

* address PR feedback

* link issue

* feat: add request response logs for sync api calls (#1747)

* fix: remove dependency on api-core for logging (#1748)

* fix: remove dep on api-core for logging

* disable propagation to the root logger

* update async helpers tests

* fix lint issue

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

api: pubsub Issues related to the Pub/Sub API. 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.

3 participants

@tseaver @dhermes @googlebot