Skip to content

Commit

Permalink
Fix mypy errors (#535)
Browse files Browse the repository at this point in the history
There were around 60 errors shown in the mypy checks about the various
minor problems across the client code base. This PR corrects
those errors and adds a check for the mypy to PR builder workflow.

Note that, for now, we are only checking the type hints in the
hazelcast directory (client's code base). Tests and examples are not
passing the mypy checks for now. We can improve this incrementally,
and add them in the future.

Also, we are not using the strict flag of the mypy, due to not
type hinting the internals yet. Hopefully, we will have that
in the future.
  • Loading branch information
mdumandag committed Mar 31, 2022
1 parent 167ca36 commit 41e9b7d
Show file tree
Hide file tree
Showing 27 changed files with 280 additions and 190 deletions.
16 changes: 16 additions & 0 deletions .github/workflows/test_runner.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,19 @@ jobs:
working-directory: docs
run: |
make html SPHINXOPTS="-W --keep-going -b linkcheck"
run-mypy:
runs-on: ubuntu-latest
name: Run mypy to check type annotations
steps:
- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: '3.10'
- name: Checkout to code
uses: actions/checkout@v2
- name: Install dependencies
run: |
pip install -r requirements-dev.txt
- name: Run mypy
run: |
mypy --show-error-codes hazelcast
6 changes: 4 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ images <https://hub.docker.com/r/hazelcast/hazelcast/>`__.

.. code:: bash
docker run -p 5701:5701 hazelcast/hazelcast:5.0
docker run -p 5701:5701 hazelcast/hazelcast:5.1
You can also use our ZIP or TAR
`distributions <https://hazelcast.com/open-source-projects/downloads/>`__.
Expand Down Expand Up @@ -204,7 +204,9 @@ If you are planning to contribute:
dependencies.
2. Use `black <https://pypi.org/project/black/>`__ to reformat the code
by running the ``black --config black.toml .`` command.
3. Make sure that tests are passing by following the steps described
3. Use `mypy <https://pypi.org/project/mypy/>`__ to check type annotations
by running the ``mypy hazelcast`` command.
4. Make sure that tests are passing by following the steps described
in the next section.

Testing
Expand Down
4 changes: 3 additions & 1 deletion docs/development_and_testing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ If you are planning to contribute:
dependencies.
2. Use `black <https://pypi.org/project/black/>`__ to reformat the code
by running the ``black --config black.toml .`` command.
3. Make sure that tests are passing by following the steps described
3. Use `mypy <https://pypi.org/project/mypy/>`__ to check type annotations
by running the ``mypy hazelcast`` command.
4. Make sure that tests are passing by following the steps described
in the :ref:`development_and_testing:testing` section.

Testing
Expand Down
2 changes: 1 addition & 1 deletion docs/getting_started.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ There are following options to start a Hazelcast cluster easily:

.. code:: bash
docker run -p 5701:5701 hazelcast/hazelcast:5.0
docker run -p 5701:5701 hazelcast/hazelcast:5.1
- You can use `Hazelcast CLI
<https://docs.hazelcast.com/hazelcast/latest/getting-started/install-hazelcast#using-a-package-manager>`__.
Expand Down
6 changes: 3 additions & 3 deletions examples/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Hazelcast Python Client Code Examples Readme
=============================
Hazelcast Python Client Code Examples Readme
============================================

This folder contains a collection of code samples which you can
use to learn how to use Hazelcast features.
Expand All @@ -14,7 +14,7 @@ How to try these examples
* If not, you can use our official Docker images to start a member.

```bash
docker run -p 5701:5701 hazelcast/hazelcast:5.0
docker run -p 5701:5701 hazelcast/hazelcast:5.1
```
To see the other ways to start Hazelcast members, see
[Working with Hazelcast Clusters](https://hazelcast.readthedocs.io/en/stable/getting_started.html#working-with-hazelcast-clusters)
Expand Down
2 changes: 1 addition & 1 deletion hazelcast/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ def _detect_membership_events(self, previous_members, current_members):
for dead_member in dead_members:
connection = self._connection_manager.get_connection(dead_member.uuid)
if connection:
connection.close(
connection.close_connection(
None,
TargetDisconnectedError(
"The client has closed the connection to this member, "
Expand Down
3 changes: 1 addition & 2 deletions hazelcast/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
check_not_none,
number_types,
LoadBalancer,
none_type,
try_to_get_enum_value,
)

Expand Down Expand Up @@ -333,7 +332,7 @@ def name(self):

@name.setter
def name(self, value):
if isinstance(value, (str, none_type)):
if isinstance(value, str) or value is None:
self._name = value
else:
raise TypeError("name must be a string or None")
Expand Down
19 changes: 10 additions & 9 deletions hazelcast/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ def shutdown(self):

# Need to create copy of connection values to avoid modification errors on runtime
for connection in list(self.active_connections.values()):
connection.close("Hazelcast client is shutting down", None)
connection.close_connection("Hazelcast client is shutting down", None)

self.active_connections.clear()
del self._connection_listeners[:]
Expand Down Expand Up @@ -613,9 +613,9 @@ def _authenticate(self, connection):
def _on_auth(self, response, connection):
try:
response = client_authentication_codec.decode_response(response.result())
except Exception as err:
connection.close("Failed to authenticate connection", err)
raise err
except Exception as e:
connection.close_connection("Failed to authenticate connection", e)
raise e

status = response["status"]
if status == _AuthenticationStatus.AUTHENTICATED:
Expand All @@ -632,7 +632,7 @@ def _on_auth(self, response, connection):
"Authentication status code not supported. status: %s" % status
)

connection.close("Failed to authenticate connection", err)
connection.close_connection("Failed to authenticate connection", err)
raise err

def _handle_successful_auth(self, response, connection):
Expand All @@ -649,7 +649,7 @@ def _handle_successful_auth(self, response, connection):

existing = self.active_connections.get(remote_uuid, None)
if existing:
connection.close(
connection.close_connection(
"Duplicate connection to same member with UUID: %s" % remote_uuid, None
)
return existing
Expand Down Expand Up @@ -750,7 +750,7 @@ def _check_client_state_on_cluster_change(self, connection):
# we can operate on. In those scenarios, we rely on the fact that we will
# reopen the connections.
reason = "Connection does not belong to the cluster %s" % self._cluster_id
connection.close(reason, None)
connection.close_connection(reason, None)
raise ValueError(reason)

def _on_cluster_restart(self):
Expand Down Expand Up @@ -824,7 +824,7 @@ def _check_connection(self, now, connection):

if (now - connection.last_read_time) > self._heartbeat_timeout:
_logger.warning("Heartbeat failed over the connection: %s", connection)
connection.close(
connection.close_connection(
"Heartbeat timed out",
TargetDisconnectedError("Heartbeat timed out to connection %s" % connection),
)
Expand Down Expand Up @@ -950,7 +950,8 @@ def send_message(self, message):
self._write(message.buf)
return True

def close(self, reason, cause):
# Not named close to distinguish it from the asyncore.dispatcher.close.
def close_connection(self, reason, cause):
"""Closes the connection.
Args:
Expand Down
14 changes: 10 additions & 4 deletions hazelcast/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ class EndpointQualifier:

__slots__ = ("_protocol_type", "_identifier")

def __init__(self, protocol_type: int, identifier: str):
def __init__(self, protocol_type: int, identifier: typing.Optional[str]):
self._protocol_type = protocol_type
self._identifier = identifier

Expand All @@ -172,7 +172,7 @@ def protocol_type(self) -> int:
return self._protocol_type

@property
def identifier(self) -> str:
def identifier(self) -> typing.Optional[str]:
"""Unique identifier for same-protocol-type endpoints."""
return self._identifier

Expand Down Expand Up @@ -492,9 +492,15 @@ def __init__(self, key: KeyType = None, value: ValueType = None):
@property
def key(self) -> KeyType:
"""Key of the entry."""
return self._key
# This has the correct type, but due to structure of the identified
# data serialization, the constructor must have ``None`` default
# values, which upsets the type checker.
return self._key # type: ignore[return-value]

@property
def value(self) -> ValueType:
"""Value of the entry."""
return self._value
# This has the correct type, but due to structure of the identified
# data serialization, the constructor must have ``None`` default
# values, which upsets the type checker.
return self._value # type: ignore[return-value]
20 changes: 12 additions & 8 deletions hazelcast/future.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ def result(self) -> ResultType:
self._event.wait()
if self._exception:
re_raise(self._exception, self._traceback)
return self._result

# Result will be set to the correct type before we
# return from here
return self._result # type: ignore[return-value]

def _reactor_check(self):
if not self.done() and hasattr(self._threading_locals, "is_reactor_thread"):
Expand Down Expand Up @@ -144,7 +147,7 @@ def continue_with(
Returns:
A new Future which will be completed when the continuation is done.
"""
future = Future()
future: Future[typing.Any] = Future()

def callback(f):
try:
Expand Down Expand Up @@ -281,8 +284,8 @@ def combine_futures(futures: typing.Sequence[Future]) -> Future:
return ImmediateFuture(results)

completed = AtomicInteger()
combined = Future()
errors = []
combined: Future[typing.List[typing.Any]] = Future()
errors: typing.List[typing.Tuple[Exception, types.TracebackType]] = []

def done(future, index):
if future.is_success():
Expand All @@ -304,9 +307,10 @@ def done(future, index):
else:
combined.set_result(results)

for index, future in enumerate(futures):
# Capture the index in the closure or else we
# will only update the last element.
future.add_done_callback(lambda f, captured_index=index: done(f, captured_index))
def make_callback(index):
return lambda f: done(f, index)

for i, future in enumerate(futures):
future.add_done_callback(make_callback(i))

return combined
4 changes: 2 additions & 2 deletions hazelcast/listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ def __init__(
self._invocation_service = invocation_service
self._compact_schema_service = compact_schema_service
self._is_smart = config.smart_routing
self._active_registrations = {} # Dict of user_registration_id, ListenerRegistration
self._active_registrations: typing.Dict[str, _ListenerRegistration] = {}
self._registration_lock = threading.RLock()
self._event_handlers = {}
self._event_handlers: typing.Dict[int, typing.Callable] = {}

def start(self):
self._connection_manager.add_listener(self._connection_added, self._connection_removed)
Expand Down
5 changes: 4 additions & 1 deletion hazelcast/proxy/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import typing

from hazelcast.invocation import Invocation
from hazelcast.protocol.codec import client_create_proxy_codec, client_destroy_proxy_codec
from hazelcast.proxy.base import Proxy
from hazelcast.proxy.executor import Executor
from hazelcast.proxy.list import List
from hazelcast.proxy.map import create_map_proxy
Expand Down Expand Up @@ -27,7 +30,7 @@
PN_COUNTER_SERVICE = "hz:impl:PNCounterService"
FLAKE_ID_GENERATOR_SERVICE = "hz:impl:flakeIdGeneratorService"

_proxy_init = {
_proxy_init: typing.Dict[str, typing.Callable[[str, str, typing.Any], Proxy]] = {
EXECUTOR_SERVICE: Executor,
LIST_SERVICE: List,
MAP_SERVICE: create_map_proxy,
Expand Down

0 comments on commit 41e9b7d

Please sign in to comment.