Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feature: Adding type annotations to Connections #1612

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feature: Adding type annotations to Connections #1612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f763078
feature: Adding type annotations to Connections
ponponon Oct 16, 2022
58804fc
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 16, 2022
716d9c0
feature: Add a type annotation to the acquire method of ConnectionPool
ponponon Oct 17, 2022
c6d006b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 17, 2022
d2edc8d
feature: add typing hint, timeout Union[int, float, None]
ponponon Oct 17, 2022
d1fdaee
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 17, 2022
29404ee
Update kombu/pools.py
auvipy Oct 17, 2022
ec39644
Update kombu/connection.py
auvipy Oct 17, 2022
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
3 changes: 3 additions & 0 deletions kombu/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,9 @@ def prepare(self, resource):
resource._debug('acquired')
return resource

def acquire(self, block: bool = False, timeout: int | float | None = None) -> Connection:
Copy link
Member

Choose a reason for hiding this comment

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

is this method newly introduced?

Copy link
Author

@ponponon ponponon Oct 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Not new, ConnectionPool inherits from Resource

Resource has an acquire method, but since Resource is an abstract class (abstracting the connection as a resource), this -> Connection type annotation should not be added to the acquire method of Resource, so I added an acquire method to ConnectionPool and added the I added an acquire method to the ConnectionPool and added the -> Connection type annotation to the ConnectionPool to make it semantic

kombu/connection.py

class ConnectionPool(Resource):
    """Pool of connections."""

    def acquire(self, block: bool = False, timeout: Union[int, float, None] = None) -> Connection:
        return super().acquire(block, timeout)

kombu/resource.py

class Resource:
    """Pool of resources."""

    def acquire(self, block=False, timeout=None):
        """Acquire resource.

        Arguments:
            block (bool): If the limit is exceeded,
                then block until there is an available item.
            timeout (float): Timeout to wait
                if ``block`` is true.  Default is :const:`None` (forever).

        Raises:
            LimitExceeded: if block is false and the limit has been exceeded.

return super().acquire(block, timeout)


class ChannelPool(Resource):
"""Pool of channels."""
Expand Down
17 changes: 13 additions & 4 deletions kombu/pools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import os
from itertools import chain

from .connection import Resource
from .connection import Connection, ConnectionPool, Resource
from .messaging import Producer
from .utils.collections import EqualityDict
from .utils.compat import register_after_fork
Expand Down Expand Up @@ -78,13 +78,13 @@ def release(self, resource):
class PoolGroup(EqualityDict):
"""Collection of resource pools."""

def __init__(self, limit=None, close_after_fork=True):
def __init__(self, limit: int | None = None, close_after_fork: bool = True):
self.limit = limit
self.close_after_fork = close_after_fork
if self.close_after_fork and register_after_fork is not None:
register_after_fork(self, _after_fork_cleanup_group)

def create(self, resource, limit):
def create(self, resource, limit: int | None):
raise NotImplementedError('PoolGroups must define ``create``')

def __missing__(self, resource):
Expand All @@ -104,9 +104,18 @@ def register_group(group):
class Connections(PoolGroup):
"""Collection of connection pools."""

def create(self, connection, limit):
def create(self, connection: Connection, limit: int | None):
return connection.Pool(limit=limit)

def __getitem__(self, connection: Connection) -> ConnectionPool:
Copy link
Member

Choose a reason for hiding this comment

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

I would like to know, what this dunder methods are going to achieve?

Copy link
Author

Choose a reason for hiding this comment

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

I have made it very clear

Code editors such as vscode, pycharm, etc. cannot provide intelligent type inference for this connections[connection].acquire(block=True) code, so I would like to add typing hint to achieve this (intelligent type inference)

Copy link
Author

Choose a reason for hiding this comment

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

When your mouse hovers over a conn, vscode will tell you that it is the type of this conn is Any, which is very stupid.

When hovering over a conn, I wish vscode would tell me what the explicit type is, instead of a stupid and damned Any

Copy link
Author

Choose a reason for hiding this comment

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

And I added these typing hint which will solve this problem very well

Copy link
Author

@ponponon ponponon Oct 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

No type hint 👇
图片

With type annotations 👇
图片

Modern program development is inseparable from the intelligent hints provided by the IDE. With these type annotations, the IDE can provide us with type inference, allowing us to program more efficiently and enjoyably

return super().__getitem__(connection)

def __setitem__(self, connection: Connection, pool: ConnectionPool):
return super().__setitem__(connection, pool)

def __delitem__(self, connection: Connection):
return super().__delitem__(connection)


connections = register_group(Connections(limit=use_global_limit))

Expand Down