Improve type annotations for distributed.core.Server
#8239
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 21 files ±0 21 suites ±0 10h 35m 11s ⏱️ - 1m 20s For more details on these failures, see this check. Results for commit e77dd56. ± Comparison against base commit 9a8b380. |
| @@ -5863,7 +5863,7 @@ def worker_send(self, worker: str, msg: dict[str, Any]) -> None: | |||
| stream_comms[worker].send(msg) | |||
| except (CommClosedError, AttributeError): | |||
| self._ongoing_background_tasks.call_soon( | |||
| self.remove_worker, | |||
| self.remove_worker, # type: ignore[arg-type] | |||
There was a problem hiding this comment.
Type annotations on the AsyncTaskGroup are too narrow.
| @@ -5909,7 +5909,7 @@ def send_all(self, client_msgs: Msgs, worker_msgs: Msgs) -> None: | |||
| pass | |||
| except (CommClosedError, AttributeError): | |||
| self._ongoing_background_tasks.call_soon( | |||
| self.remove_worker, | |||
| self.remove_worker, # type: ignore[arg-type] | |||
There was a problem hiding this comment.
Type annotations on the AsyncTaskGroup are too narrow.
| @@ -673,13 +719,13 @@ def stop(self) -> None: | |||
| self.monitor.close() | |||
| if not (stop_listeners := self._stop_listeners()).done(): | |||
| self._ongoing_background_tasks.call_soon( | |||
| asyncio.wait_for(stop_listeners, timeout=None) | |||
| asyncio.wait_for(stop_listeners, timeout=None) # type: ignore[arg-type] | |||
There was a problem hiding this comment.
Type annotations on the AsyncTaskGroup are too narrow. This should be addressed in a follow-up PR.
| @@ -784,7 +831,8 @@ def _to_dict(self, *, exclude: Container[str] = ()) -> dict: | |||
| Client.dump_cluster_state | |||
| distributed.utils.recursive_to_dict | |||
| """ | |||
| info = self.identity() | |||
| info: dict = {} | |||
| info.update(self.identity()) | |||
There was a problem hiding this comment.
I think it's more correct to change the return type to dict[str, Any] instead
| @@ -963,7 +1011,9 @@ async def _handle_comm(self, comm): | |||
| "Failed while closing connection to %r: %s", address, e | |||
| ) | |||
|
|
|||
| async def handle_stream(self, comm, extra=None): | |||
| async def handle_stream( | |||
| self, comm: Comm, extra: dict[str, str] | None = None | |||
There was a problem hiding this comment.
| self, comm: Comm, extra: dict[str, str] | None = None | |
| self, comm: Comm, extra: dict[str, Any] | None = None |
extra are arbitrary kwargs to stream_handlers - I can't see a reason to limit them to str values?
| def get_monitor_info( | ||
| self, recent: bool = False, start: int = 0 | ||
| ) -> dict[str, float]: | ||
| # FIXME: Improve typing |
There was a problem hiding this comment.
| # FIXME: Improve typing |
I think that precisely specifying the possible types of the return dict values would actually do harm.
See python/typing#566
|
I was too late - my review comments are now in #8247 |
Closes #xxxx
pre-commit run --all-files