fix: improve type checking#2530
Conversation
There was a problem hiding this comment.
Code Review
This pull request successfully addresses the issues identified with mypy by unpinning its version and resolving the type-related problems it exposed. The changes include adding type hints for better clarity, handling None values in request objects, and improving logging for debugging. The addition of --check-untyped-defs to the mypy configuration enhances the strictness of type checking, which is a positive step for code quality. Overall, the changes improve the robustness and maintainability of the generated code.
| if request is None: | ||
| request = {} |
There was a problem hiding this comment.
| if request is None: | ||
| request = {} |
There was a problem hiding this comment.
| if request is None: | ||
| request = {} |
There was a problem hiding this comment.
| if request is None: | ||
| request = {} |
There was a problem hiding this comment.
| if request is None: | ||
| request = {} |
There was a problem hiding this comment.
| if request is None: | ||
| request = {} |
There was a problem hiding this comment.
| "--check-untyped-defs", | ||
| *session.posargs, |
There was a problem hiding this comment.
There was a problem hiding this comment.
There should be a blank line before this decorator to separate the tests.
This happens multiple times in the PR.
There was a problem hiding this comment.
Good catch. It looks like this file was generating other methods with that same problem, even before this PR. I'm kind of surprised it's not raising any errors...
But I updated the template, so this shouldn't be a problem now
chalmerlowe
left a comment
There was a problem hiding this comment.
I have one comment: it looks like something is off in one of the templates OR the generation code block that is causing a test to appear immediately after another test with no intervening blank line.
Linchin
left a comment
There was a problem hiding this comment.
Looks good to me, seems mostly to be type annotation improvements with some additional unit test templates. But I think it makes sense to let someone with more gapic expertise to double check the tests.

Fixes #2410
We currently have mypy pinned to an older version, because it fails on newer ones
This PR unpins the dependency to use the latest version, and fixes the problems it finds:
*_operationmethods in _mixins accept None values, but don't properly handle the None case_get_default_mtls_endpointdoesn't handle what happens if the regex check returns None