Skip to content

Commit 2cdfa75

Browse files
petyaslavovapetyaslavova
committed
Applying review comments.
1 parent a82cbfe commit 2cdfa75

File tree

4 files changed

+51
-61
lines changed

4 files changed

+51
-61
lines changed

‎redis/asyncio/connection.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,8 +1308,6 @@ def __init__(
13081308
)
13091309
self._condition = asyncio.Condition()
13101310
self.timeout = timeout
1311-
self._in_maintenance = False
1312-
self._locked = False
13131311

13141312
@deprecated_args(
13151313
args_to_warn=["*"],

‎redis/connection.py‎

Lines changed: 42 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,28 +1906,36 @@ def re_auth_callback(self, token: TokenInterface):
19061906
for conn in self._in_use_connections:
19071907
conn.set_re_auth_token(token)
19081908

1909+
def should_update_connection(
1910+
self,
1911+
conn: "Connection",
1912+
address_type_to_match: Literal["connected", "configured"] = "connected",
1913+
matching_address: Optional[str] = None,
1914+
) -> bool:
1915+
if address_type_to_match == "connected":
1916+
if matching_address and conn.getpeername() != matching_address:
1917+
return False
1918+
else:
1919+
if matching_address and conn.host != matching_address:
1920+
return False
1921+
return True
1922+
19091923
def set_maintenance_state_for_connections(
19101924
self,
19111925
state: "MaintenanceState",
19121926
matching_address: Optional[str] = None,
19131927
address_type_to_match: Literal["connected", "configured"] = "connected",
19141928
):
19151929
for conn in self._available_connections:
1916-
if address_type_to_match == "connected":
1917-
if matching_address and conn.getpeername() != matching_address:
1918-
continue
1919-
else:
1920-
if matching_address and conn.host != matching_address:
1921-
continue
1922-
conn.maintenance_state = state
1930+
if self.should_update_connection(
1931+
conn, address_type_to_match, matching_address
1932+
):
1933+
conn.maintenance_state = state
19231934
for conn in self._in_use_connections:
1924-
if address_type_to_match == "connected":
1925-
if matching_address and conn.getpeername() != matching_address:
1926-
continue
1927-
else:
1928-
if matching_address and conn.host != matching_address:
1929-
continue
1930-
conn.maintenance_state = state
1935+
if self.should_update_connection(
1936+
conn, address_type_to_match, matching_address
1937+
):
1938+
conn.maintenance_state = state
19311939

19321940
def set_maintenance_state_in_connection_kwargs(self, state: "MaintenanceState"):
19331941
self.connection_kwargs["maintenance_state"] = state
@@ -2091,23 +2099,17 @@ def update_connections_current_timeout(
20912099
:param include_available_connections: Whether to include available connections in the update.
20922100
"""
20932101
for conn in self._in_use_connections:
2094-
if address_type_to_match == "connected":
2095-
if matching_address and conn.getpeername() != matching_address:
2096-
continue
2097-
else:
2098-
if matching_address and conn.host != matching_address:
2099-
continue
2100-
conn.update_current_socket_timeout(relax_timeout)
2102+
if self.should_update_connection(
2103+
conn, address_type_to_match, matching_address
2104+
):
2105+
conn.update_current_socket_timeout(relax_timeout)
21012106

21022107
if include_free_connections:
21032108
for conn in self._available_connections:
2104-
if address_type_to_match == "connected":
2105-
if matching_address and conn.getpeername() != matching_address:
2106-
continue
2107-
else:
2108-
if matching_address and conn.host != matching_address:
2109-
continue
2110-
conn.update_current_socket_timeout(relax_timeout)
2109+
if self.should_update_connection(
2110+
conn, address_type_to_match, matching_address
2111+
):
2112+
conn.update_current_socket_timeout(relax_timeout)
21112113

21122114
def _update_connection_for_reconnect(
21132115
self,
@@ -2439,24 +2441,18 @@ def update_connections_current_timeout(
24392441
"""
24402442
if include_free_connections:
24412443
for conn in tuple(self._connections):
2442-
if address_type_to_match == "connected":
2443-
if matching_address and conn.getpeername() != matching_address:
2444-
continue
2445-
else:
2446-
if matching_address and conn.host != matching_address:
2447-
continue
2448-
conn.update_current_socket_timeout(relax_timeout)
2444+
if self.should_update_connection(
2445+
conn, address_type_to_match, matching_address
2446+
):
2447+
conn.update_current_socket_timeout(relax_timeout)
24492448
else:
24502449
connections_in_queue = {conn for conn in self.pool.queue if conn}
24512450
for conn in self._connections:
24522451
if conn not in connections_in_queue:
2453-
if address_type_to_match == "connected":
2454-
if matching_address and conn.getpeername() != matching_address:
2455-
continue
2456-
else:
2457-
if matching_address and conn.host != matching_address:
2458-
continue
2459-
conn.update_current_socket_timeout(relax_timeout)
2452+
if self.should_update_connection(
2453+
conn, address_type_to_match, matching_address
2454+
):
2455+
conn.update_current_socket_timeout(relax_timeout)
24602456

24612457
def _update_maintenance_events_config_for_connections(
24622458
self, maintenance_events_config
@@ -2508,11 +2504,7 @@ def set_maintenance_state_for_connections(
25082504
address_type_to_match: Literal["connected", "configured"] = "connected",
25092505
):
25102506
for conn in self._connections:
2511-
if address_type_to_match == "connected":
2512-
if matching_address and conn.getpeername() != matching_address:
2513-
continue
2514-
else:
2515-
if matching_address and conn.host != matching_address:
2516-
continue
2517-
2518-
conn.maintenance_state = state
2507+
if self.should_update_connection(
2508+
conn, address_type_to_match, matching_address
2509+
):
2510+
conn.maintenance_state = state

‎redis/maintenance_events.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ def handle_node_moving_event(self, event: NodeMovingEvent):
406406
)
407407
if getattr(self.pool, "set_in_maintenance", False):
408408
self.pool.set_in_maintenance(False)
409-
print(f"Starting timer for {event} for {event.ttl} seconds")
409+
410410
threading.Timer(
411411
event.ttl, self.handle_node_moved_event, args=(event,)
412412
).start()

‎tests/test_maintenance_events_handling.py‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ def validate_in_use_connections_state(
7373
def validate_free_connections_state(
7474
pool,
7575
should_be_connected_count=0,
76-
connected_to_tmp_addres=False,
76+
connected_to_tmp_address=False,
7777
tmp_address=AFTER_MOVING_ADDRESS.split(":")[0],
7878
expected_state=MaintenanceState.MOVING,
7979
expected_host_address=DEFAULT_ADDRESS.split(":")[0],
@@ -107,7 +107,7 @@ def validate_free_connections_state(
107107
assert connection.maintenance_state == expected_state
108108
if connection._sock is not None:
109109
assert connection._sock.connected is True
110-
if connected_to_tmp_addres and tmp_address != "any":
110+
if connected_to_tmp_address and tmp_address != "any":
111111
assert connection._sock.getpeername()[0] == tmp_address
112112
connected_count += 1
113113
assert connected_count == should_be_connected_count
@@ -870,7 +870,7 @@ def test_moving_related_events_handling_integration(self, pool_class):
870870
expected_orig_socket_timeout=None,
871871
expected_orig_socket_connect_timeout=None,
872872
should_be_connected_count=1,
873-
connected_to_tmp_addres=True,
873+
connected_to_tmp_address=True,
874874
)
875875
# Wait for MOVING timeout to expire and the moving completed handler to run
876876
sleep(MOVING_TIMEOUT + 0.5)
@@ -906,7 +906,7 @@ def test_moving_related_events_handling_integration(self, pool_class):
906906
expected_orig_socket_timeout=None,
907907
expected_orig_socket_connect_timeout=None,
908908
should_be_connected_count=1,
909-
connected_to_tmp_addres=True,
909+
connected_to_tmp_address=True,
910910
expected_state=MaintenanceState.NONE,
911911
)
912912
finally:
@@ -1227,7 +1227,7 @@ def test_overlapping_moving_events(self, pool_class):
12271227
Helpers.validate_free_connections_state(
12281228
pool=test_redis_client.connection_pool,
12291229
should_be_connected_count=1,
1230-
connected_to_tmp_addres=True,
1230+
connected_to_tmp_address=True,
12311231
expected_state=MaintenanceState.MOVING,
12321232
expected_host_address=AFTER_MOVING_ADDRESS.split(":")[0],
12331233
expected_socket_timeout=self.config.relax_timeout,
@@ -1279,7 +1279,7 @@ def test_overlapping_moving_events(self, pool_class):
12791279
Helpers.validate_free_connections_state(
12801280
test_redis_client.connection_pool,
12811281
should_be_connected_count=1,
1282-
connected_to_tmp_addres=True,
1282+
connected_to_tmp_address=True,
12831283
tmp_address=second_moving_address.split(":")[0],
12841284
expected_state=MaintenanceState.MOVING,
12851285
expected_host_address=second_moving_address.split(":")[0],
@@ -1401,7 +1401,7 @@ def test_moving_migrating_migrated_moved_state_transitions(self, pool_class):
14011401
Helpers.validate_free_connections_state(
14021402
pool=pool,
14031403
should_be_connected_count=0,
1404-
connected_to_tmp_addres=False,
1404+
connected_to_tmp_address=False,
14051405
expected_state=MaintenanceState.MOVING,
14061406
expected_host_address=tmp_address,
14071407
expected_socket_timeout=self.config.relax_timeout,
@@ -1465,7 +1465,7 @@ def test_moving_migrating_migrated_moved_state_transitions(self, pool_class):
14651465
Helpers.validate_free_connections_state(
14661466
pool=pool,
14671467
should_be_connected_count=0,
1468-
connected_to_tmp_addres=False,
1468+
connected_to_tmp_address=False,
14691469
expected_state=MaintenanceState.NONE,
14701470
expected_host_address=DEFAULT_ADDRESS.split(":")[0],
14711471
expected_socket_timeout=None,

0 commit comments

Comments
 (0)