#861: Implement 'pubsub.client.Client'.

[tseaver]

Wraps:

• 'pubsub.api' functions
• 'pubsub.topic.Topic' (via a factory / proxy)
• 'pubsub.subscription.Subscription' (via a factory / proxy)

See:
#861 (comment)

[coveralls]

Coverage remained the same at 100.0% when pulling 4793f75 on tseaver:pubsub-client_w_proxies into 6bc4e02 on GoogleCloudPlatform:master.

[dhermes]

1. ISTM there is too much going on here to be considered 1 PR / 1 issue being addressed. In fact, it changes things I didn't know needed changing.
2. Can you squash the lint picking commit? Picking the lint before sending the PR / making a commit fixes your need to make such commits.

[coveralls]

Coverage remained the same at 100.0% when pulling 9b4a15e on tseaver:pubsub-client_w_proxies into 6bc4e02 on GoogleCloudPlatform:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 42889a5 on tseaver:pubsub-client_w_proxies into 6cf0852 on GoogleCloudPlatform:master.

[tseaver]

FWIW, I've written a couple of example "multi-tenant" scripts, one showing using the Client and one showing the using the API directly.

I honestly can't see much win for the client pattern there, but it should make it clear how it will be used.

[tseaver]

@dhermes

ISTM there is too much going on here to be considered 1 PR / 1 issue being addressed. In fact, it changes things I didn't know needed changing.

I'm not sure I understand the objection: this PR doesn't change any existing code -- it only adds code to implement the Client pattern for pubsub.

[dhermes]

The objection is to the the wrapper / proxy approach (and the need to get fancy with __getattr__ for the proxy classes).

Can we design this thing before writing it?

Currently we are introduce code like

>>>topic=client.topic(...)
>>>isinstance(topic, Topic)
False

and that just isn't great.

---

To maintain the OO approach, we may be able to bind the clients to the objects and just do a better job handling batches/transactions.

[tseaver]

I'm still firmly convinced that forcing the client pattern on all users the library is a mistake: leaving it as a convenience layer "above" the current implementation allows us to experiment with the pattern without committing to it as the One True Way to use the libraries.

[dhermes]

Well let's have that discussion before writing 500+ lines of code.

[tseaver]

500 lines of code? The 568 lines in the patch are 90+ lines of docs, 310+ lines of unit tests, ~30 liines of module boilerplate, and 130 lines of "real" code. Those 130 offer a working proof-of-concept that a layered approach is workable. We can argue about the merits of proxies (DRY, easier mainteinance) vs. explicitly wrapped APIs (better tooling support?), but it is possible to offer the pattern as an option, without imposing it willy-nilly on all uses of the library.

The discussion we were having seemed to me unresolved / ungrounded -- it was all based on comparing tiny snippets, without being able to compare how the new pattern would look in real applications. I picked pubsub as a sample, because it was the simplest, and wrote this PR to show that we could offer both patterns in a layered approach, including being able to compare real-world examples of both. For instance, both gists I posted earlier today will work with the code in this PR.

[dhermes]

Sorry, I was going off

the LOC thing was not particular helpful of me to point out.

I agree this is the best place to test the client pattern.

I don't think the proxies are a good fix and I do think we will likely need to just ditch the _implicit_environ behavior. People in general are un-easy about defaults that they can't see, so by having the defaults set at client instantiation we alleviate those concerns (and a fair amount of complexity).

[coveralls]

Coverage remained the same at 100.0% when pulling cc086b6 on tseaver:pubsub-client_w_proxies into 6cf0852 on GoogleCloudPlatform:master.

[dhermes]

@tseaver I'm closing this given #910 being in. Please re-open if you think necessary.