Skip to content

feat: add support for multiprocessing.resource_sharer type hints #8413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 28, 2022
Merged
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions stdlib/multiprocessing/resource_sharer.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import sys
from collections.abc import Callable
from socket import socket
from typing import Union
from typing_extensions import TypeAlias

from .connection import Connection

__all__ = ["stop"]

# https://docs.python.org/3/library/multiprocessing.html#address-formats
_Address: TypeAlias = Union[str, tuple[str, int]]

if sys.platform == "win32":
__all__ += ["DupSocket"]

class DupSocket:
def __init__(self, sock: socket) -> None: ...
def detach(self) -> bytes: ...

else:
__all__ += ["DupFd"]

class DupFd:
def __init__(self, fd: int) -> None: ...
def detach(self) -> None: ...

class _ResourceSharer:
def __init__(self) -> None: ...
def register(self, send: Callable[[Connection, int], None], close: Callable[[], None]) -> tuple[_Address, int]: ...
@staticmethod
def get_connection(ident: tuple[_Address, int]) -> Connection: ...
def stop(self, timeout: float | None = ...) -> None: ...

_resource_sharer: _ResourceSharer = ...
stop = _resource_sharer.stop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks like a bit too much to maintain, considering it's a private class in an undocumented module. The only public think here is the stop() callable:

Suggested change
class _ResourceSharer:
def __init__(self) -> None: ...
def register(self, send: Callable[[Connection, int], None], close: Callable[[], None]) -> tuple[_Address, int]: ...
@staticmethod
def get_connection(ident: tuple[_Address, int]) -> Connection: ...
def stop(self, timeout: float | None = ...) -> None: ...
_resource_sharer: _ResourceSharer = ...
stop = _resource_sharer.stop
def stop(timeout: float | None = ...) -> None: ...

Copy link
Contributor Author

@kkirsche kkirsche Jul 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the point, though personally I think it's valuable to provide these types of things for developers who choose to start doing development on internals in more depth, as without the private methods, for me at least, it really hinders learning the cpython stdlib codebase

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that typeshed is a fantastic tool for learning about the stdlib codebase, but that's ultimately not its primary purpose.

The issue with including implementation details in the stub is that there are no backwards compatibility guarantees, so they're liable to change frequently, possibly even in micro releases of CPython. That just imposes too much of a maintenance burden on us as maintainers.

There's also a question of the usefulness of these implementation details for typechecking. End users of this module shouldn't be using implementation details of this module in their applications or libraries (because they're liable to change at any time), so it's good, in a way, if a typechecker emits an error when a user tries to import one of these implementation details.