Skip to content

Commit 51d370f

Browse files
committed
fix: remove CLIENT REPLY, CLIENT UNBLOCK, and QUIT
These three commands are incompatible with (or deprecated for) GLIDE's architecture and no other GLIDE client exposes them: - CLIENT REPLY toggles reply-mode state that glide-core does not track, and on the multiplexed connection it desynchronizes request/response matching for every caller sharing the connection. - CLIENT UNBLOCK acts on another client by ID, but GLIDE manages blocking commands internally and does not expose the connection IDs needed to use it, so there is no supported use case. - QUIT is deprecated server-side; GLIDE owns the connection lifecycle, and closing the shared multiplexed connection is harmful. Users should close the client object instead. Remove the methods and their lint tests entirely so they cannot be used. Fixes #183 Signed-off-by: Alex Le <alex.le@improving.com>
1 parent 0c81c7d commit 51d370f

3 files changed

Lines changed: 0 additions & 65 deletions

File tree

lib/valkey/commands/connection_commands.rb

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -47,25 +47,6 @@ def select(db)
4747
send_command(RequestType::SELECT, [db])
4848
end
4949

50-
# Close the connection.
51-
#
52-
# @deprecated The QUIT command is deprecated since Redis 7.2.0 / Valkey 7.2+.
53-
# Clients should use the `close` method directly instead.
54-
# This avoids lingering TIME_WAIT sockets on the server side.
55-
#
56-
# @return [String] `OK` or nil if connection already closed
57-
# @see https://redis.io/docs/latest/commands/quit/
58-
def quit
59-
# For compatibility, we still support QUIT but recommend using close() instead
60-
send_command(RequestType::QUIT)
61-
rescue ConnectionError
62-
# Server closes connection immediately after QUIT
63-
nil
64-
ensure
65-
# Clean up our side of the connection
66-
close if respond_to?(:close)
67-
end
68-
6950
# Switch to a different protocol version and handshake with the server.
7051
#
7152
# @param [Integer] protover Protocol version (2 or 3)
@@ -219,25 +200,6 @@ def client_unpause(route: nil)
219200
send_command(RequestType::CLIENT_UNPAUSE, [], route: route)
220201
end
221202

222-
# Configure client reply mode.
223-
#
224-
# @param [String] mode Reply mode (ON, OFF, SKIP)
225-
# @return [String] `OK`
226-
def client_reply(mode)
227-
send_command(RequestType::CLIENT_REPLY, [mode])
228-
end
229-
230-
# Unblock a client blocked in a blocking operation.
231-
#
232-
# @param [Integer] client_id ID of the client to unblock
233-
# @param [String] unblock_type Optional unblock type (TIMEOUT, ERROR)
234-
# @return [Integer] 1 if client was unblocked, 0 otherwise
235-
def client_unblock(client_id, unblock_type = nil)
236-
args = [client_id]
237-
args << unblock_type if unblock_type
238-
send_command(RequestType::CLIENT_UNBLOCK, args)
239-
end
240-
241203
# Set client connection information.
242204
#
243205
# @param [String] attr Attribute to set (lib-name, lib-ver)

test/lint/connection_commands.rb

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,6 @@ def test_client_pause_unpause
108108
assert_equal "OK", r.client(:unpause)
109109
end
110110

111-
def test_client_reply
112-
# Use the server commands interface that's known to work
113-
assert_equal "OK", r.client(:reply, "ON")
114-
end
115-
116111
def test_client_set_info
117112
target_version "7.2" do
118113
# Use the server commands interface that's known to work
@@ -123,13 +118,6 @@ def test_client_set_info
123118
end
124119
end
125120

126-
def test_client_unblock
127-
# Use the server commands interface that's known to work
128-
client_id = r.client(:id)
129-
result = r.client(:unblock, client_id)
130-
assert [0, 1].include?(result), "Unblock should return 0 or 1"
131-
end
132-
133121
def test_client_no_evict
134122
# Use the server commands interface that's known to work
135123
assert_equal "OK", r.client_no_evict(:on)
@@ -195,11 +183,5 @@ def test_reset
195183
end
196184
assert_nil r.client_get_name
197185
end
198-
199-
def test_quit
200-
# NOTE: This test is tricky because QUIT closes the connection
201-
# We'll skip it in lint tests to avoid connection issues
202-
skip("QUIT command closes connection - tested separately")
203-
end
204186
end
205187
end

test/lint/server_commands.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,15 +156,6 @@ def test_client_set_info
156156
end
157157
end
158158

159-
def test_client_unblock
160-
result = r.client(:unblock, r.client(:id))
161-
assert [0, 1].include?(result), "Expected unblock to return 0 or 1"
162-
end
163-
164-
def test_client_reply
165-
assert_equal "OK", r.client(:reply, "ON") # TODO: "OFF" or "SKIP" doesnt work yet
166-
end
167-
168159
def test_client_kill
169160
# CLIENT KILL by address doesn't work reliably in cluster mode because
170161
# the command may be routed to a different node than where the client is connected

0 commit comments

Comments
 (0)