Skip to content

Conversation

@roberfi
Copy link
Contributor

@roberfi roberfi commented Nov 25, 2025

Add missing types for the methods of the following classes:

  • Container
  • ContainerCollection
  • ExecApiMixin

References:

- `Container`
- `ContainerCollection`
- `ExecApiMixin`
@roberfi roberfi marked this pull request as ready for review November 25, 2025 15:48
@github-actions

This comment has been minimized.

): ...
self,
exec_id: str,
detach: Literal[True] = ...,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This wouldn't work, since it would also match if detach is left out (meaning it's False):

Suggested change
detach: Literal[True] = ...,
detach: Literal[True],

Comment on lines 36 to 45
@overload
def exec_start(
self,
exec_id: str,
detach: Literal[False] = False,
tty: bool = False,
stream: bool = False,
socket: Literal[True] = ...,
demux: bool = False,
) -> SocketIO | _BufferedReaderStream | SSHSocket: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same is true for the socket argument here. Unfortunately, we can't just remove the default as that would cause a syntax error. So we can to split this overload into two:

Suggested change
@overload
def exec_start(
self,
exec_id: str,
detach: Literal[False] = False,
tty: bool = False,
stream: bool = False,
socket: Literal[True] = ...,
demux: bool = False,
) -> SocketIO | _BufferedReaderStream | SSHSocket: ...
@overload
def exec_start(
self,
exec_id: str,
detach: Literal[False],
tty: bool,
stream: bool,
socket: Literal[True],
demux: bool = False,
) -> SocketIO | _BufferedReaderStream | SSHSocket: ...
@overload
def exec_start(
self,
exec_id: str,
detach: Literal[False] = False,
tty: bool = False,
stream: bool = False,
*,
socket: Literal[True],
demux: bool = False,
) -> SocketIO | _BufferedReaderStream | SSHSocket: ...

The same for the other overloads using a ... default below. I won't be pretty.

@roberfi
Copy link
Contributor Author

roberfi commented Nov 26, 2025

Thank you for your hints @srittau. I think that overloads work properly now

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants