PYTHON-3813 add types to pool.py #1318
Conversation
|
Sorry could you fix the merge conflicts and then I'll review this asap? |
|
yeah, just fixed merge conflicts! Sorry, I didn't notice them earlier |
| self.socket_checker = SocketChecker() | ||
| self.oidc_token_gen_id = None | ||
| self.compression_context: Union[SnappyContext, ZlibContext, ZstdContext, None] = None | ||
| self.socket_checker: SocketChecker = SocketChecker() |
There was a problem hiding this comment.
Is SocketChecker necessary here? It should be redundant here right?
There was a problem hiding this comment.
So this one is interesting. I agree that it feels redundant but without it mypy fails. I can take it away so you can see what the test failure would look like if that'd help.
There was a problem hiding this comment.
just did this commit.
There was a problem hiding this comment.
So the error this? What does SocketChecker have to do with this?:
Found 2 errors in 1 file (checked 85 source files)
pymongo/pool.py: note: In function "_configured_socket":
pymongo/pool.py:12[42](https://github.com/mongodb/mongo-python-driver/actions/runs/5757722911/job/15609185080#step:5:43): error: Incompatible types in assignment (expression has
type "_sslConn", variable has type "socket") [assignment]
sock = ssl_context.wrap_socket(sock, server_hostname=h...
^
pymongo/pool.py:12[44](https://github.com/mongodb/mongo-python-driver/actions/runs/5757722911/job/15609185080#step:5:45): error: Incompatible types in assignment (expression has
type "_sslConn", variable has type "socket") [assignment]
sock = ssl_context.wrap_socket(sock)
^
typecheck-mypy: exit 1 (14.39 seconds) /home/runner/work/mongo-python-driver/mongo-python-driver> mypy --install-types --non-interactive bson gridfs tools pymongo pid=1865
There was a problem hiding this comment.
wait interesting, the error i get when i run it locally is this:
typecheck-mypy: commands[1]> mypy --install-types --non-interactive --disable-error-code var-annotated --disable-error-code attr-defined --disable-error-code union-attr --disable-error-code assignment --disable-error-code no-redef --disable-error-code index --allow-redefinition --allow-untyped-globals --exclude 'test/mypy_fails/*.*' --exclude test/conftest.py test
Found 1 error in 1 file (checked 125 source files)
pymongo/pool.py:83: note: In module imported here,
test/pymongo_mocks.py:26: note: ... from here:
pymongo/network.py: note: In function "wait_for_read":
pymongo/network.py:303: error: Cannot determine type of "socket_checker" [has-type]
readable = conn.socket_checker.select(sock, read=True, timeout=timeout)
^~~~~~~~~~~~~~~~~~~
typecheck-mypy: exit 1 (10.22 seconds) /Users/iris.ho/GitHub/mongo-python-driver> mypy --install-types --non-interactive --disable-error-code var-annotated --disable-error-code attr-defined --disable-error-code union-attr --disable-error-code assignment --disable-error-code no-redef --disable-error-code index --allow-redefinition --allow-untyped-globals --exclude 'test/mypy_fails/*.*' --exclude test/conftest.py test pid=95467
.pkg: _exit> python /Users/iris.ho/Library/Python/3.9/lib/python/site-packages/pyproject_api/_backend.py True setuptools.build_meta
typecheck-mypy: FAIL code 1 (18.80=setup[6.96]+cmd[1.62,10.22] seconds)
evaluation failed :( (18.87 seconds)
There was a problem hiding this comment.
Hmm when I run tox locally I get a different error:
pymongo/pool.py:83: note: In module imported here,
test/pymongo_mocks.py:26: note: ... from here:
pymongo/network.py: note: In function "wait_for_read":
pymongo/network.py:303: error: Cannot determine type of "socket_checker" [has-type]
readable = conn.socket_checker.select(sock, read=True, timeout=timeout)
^~~~~~~~~~~~~~~~~~~
Found 1 error in 1 file (checked 125 source files)
There was a problem hiding this comment.
Yeah it does look like adding SocketChecker as a type here is required.
bson/typings.py
Outdated
| @@ -24,6 +24,6 @@ | |||
|
|
|||
| # Common Shared Types. | |||
| _DocumentOut = Union[MutableMapping[str, Any], "RawBSONDocument"] | |||
| _DocumentType = TypeVar("_DocumentType", bound=Mapping[str, Any]) | |||
| _DocumentType = TypeVar("_DocumentType", bound=Mapping[str, Any], covariant=True) | |||
There was a problem hiding this comment.
Hmm why does this need covariant=True now?
There was a problem hiding this comment.
pymongo/pool.py
Outdated
| @@ -808,7 +860,7 @@ def _hello(self, cluster_time, topology_version, heartbeat_frequency): | |||
| self.generation = self.pool_gen.get(self.service_id) | |||
| return hello | |||
|
|
|||
| def _next_reply(self): | |||
| def _next_reply(self) -> Mapping[str, Any]: | |||
There was a problem hiding this comment.
Let's change this to Dict[str, Any], then we can remove the covariant=True.
There was a problem hiding this comment.
Sounds good, done.
https://jira.mongodb.org/browse/PYTHON-3813