fix: refactor client classes for safer type checking

[plamut]

Towards #536.

This PR is based on #551, it addresses the false positives mypy produces due to the fact that it cannot know that most of the generated methods are dynamically injected into the hand-written client wrapper classes.

#551 should likely be merged first, as this change is somewhat more risky, and we want to have it in its own commit to make the rollback easier and smaller, should that be necessary.

[tswast]

Looks a lot cleaner! A couple of questions.

[plamut]

@tswast Applied the suggestions.

Sorry for the force push, rebased just the relevant commits onto the latest main after #551 was merged.

[tswast]

Getting a few mypy errors on the samples

nox > Running session mypy_samples
nox > Creating virtual environment (virtualenv) using python3.8 in .nox/mypy_samples
nox > python -m pip install -e '.[all]'
nox > python -m pip install pytest
nox > python -m pip install mypy==0.910
nox > python -m pip install types-mock types-protobuf types-setuptools
nox > mypy --config-file /tmpfs/src/github/python-pubsub/samples/snippets/mypy.ini --no-incremental samples/
samples/snippets/utilities/us_states_pb2.py:11: error: Call to untyped function "Default" in typed context
samples/snippets/subscriber.py:107: error: Module "google.cloud.pubsub_v1.types" has no attribute "DeadLetterPolicy"
samples/snippets/subscriber.py:172: error: Module has no attribute "PushConfig"
samples/snippets/subscriber.py:260: error: Module has no attribute "PushConfig"
samples/snippets/subscriber.py:262: error: Module has no attribute "Subscription"
samples/snippets/subscriber.py:290: error: Module "google.cloud.pubsub_v1.types" has no attribute "DeadLetterPolicy"
samples/snippets/subscriber.py:290: error: Module "google.cloud.pubsub_v1.types" has no attribute "FieldMask"
samples/snippets/subscriber.py:330: error: Module has no attribute "Subscription"
samples/snippets/subscriber.py:335: error: Redundant cast to "Subscription"
samples/snippets/subscriber.py:353: error: Module "google.cloud.pubsub_v1.types" has no attribute "FieldMask"
samples/snippets/subscriber.py:378: error: Module has no attribute "Subscription"
samples/snippets/subscriber.py:383: error: Redundant cast to "Subscription"
samples/snippets/subscriber.py:585: error: Call to untyped function "Retry" in typed context
samples/snippets/subscriber.py:631: error: Call to untyped function "Retry" in typed context
samples/snippets/publisher.py:281: error: Call to untyped function "Retry" in typed context
samples/snippets/publisher.py:286: error: Call to untyped function "if_exception_type" in typed context
samples/snippets/subscriber_test.py:179: error: Module "google.cloud.pubsub_v1.types" has no attribute "DeadLetterPolicy"
Found 17 errors in 4 files (checked 13 source files)
nox > Command mypy --config-file /tmpfs/src/github/python-pubsub/samples/snippets/mypy.ini --no-incremental samples/ failed with exit code 1
nox > Session mypy_samples failed.

[tswast]

LGTM other than the failing samples checks. Thanks for adding the api property back.

[tswast]

Out-of-date comment?

[plamut]

Indeed, will remove it so that owlbot will not add it back.

[plamut]

Getting a few mypy errors on the samples

This used to pass (IIRC), will see what changes are needed to bring it back to green state.

[plamut]

@tswast I confused the type-check errors with BigQuery, they were only fixed there.

There are two classes of errors reported by Pub/Sub samples type check:

• Generated proto types are injected into google.cloud.pubsub_v1.types dynamically at import time, and mypy cannot know about them.

  Module has no attribute "ProtoMessageType"
  

• Untyped objects from python-api-core - not all of them have been annotated, for example api_core.retry.if_exception_type and api_core.retry.Retry from the retry module.

  Call to untyped function "if_exception_type" in typed context
  

While the latter can be solved by adding the missing annotations to api-core, the former would require quite some acrobatics to make it work.

We would have to avoid dynamically injected content, perhaps by some sort of a preprocessor that would copy/paste the generated types directly into google/cloud/pubsub_v1/types.py. In principle doable, but would require changes to the process and an update to tooling, which is too large a task to fit into this PR... 😕

cc: @pradn

---

Edit:
There's also a third option - let's keep the mypy_samples session commented out and at least merge the refactoring improvements. With that we would only have two classes of errors (described above) left to deal with.

I'd say this gives us a good base for future iterations, while preserving the improvements accumulated so far.

[tswast]

While the latter can be solved by adding the missing annotations to api-core, the former would require quite some acrobatics to make it work.

I think we should definitely annotate the missing methods in api-core regardless of next steps.

We would have to avoid dynamically injected content, perhaps by some sort of a preprocessor that would copy/paste the generated types directly into google/cloud/pubsub_v1/types.py

That is probably the right long-term solution. That said, it appears there are less than a dozen instances of "Module has no attribute" in the mypy_samples run. What if we use google.pubsub_v1.types instead of google.cloud.pubsub_v1.types in the code samples?

https://github.com/googleapis/python-pubsub/blob/main/google/pubsub_v1/types/__init__.py

The latter does have a few manual classes, but for the proto message types seems to just be redirecting (way too many in my opinion, there's no point in defining aliases for the shared types for example) various protobuf modules into the same namespace.

[tswast]

Re: third option, since we haven't added a py.typed file to https://github.com/googleapis/python-pubsub/tree/main/google/cloud/pubsub_v1 yet, that'd be okay with me, as this is an improvement. We do need mypy_samples as a blocker for that, so let's make sure we cross-link #536 with the todo to uncomment mypy_samples.