[troyan-dy]

Pull Request check-list

• Do tests and lints pass with this change? (See below for what I ran.)
• Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
• Is the new or changed code fully tested?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)? (Docstrings updated; no API change.)
• Is there an example added to the examples folder (if applicable)? (Not applicable.)

Description of change

Fixes #307.

sentinel_ckquorum() always returned False when the quorum was reachable,
and raised ResponseError when it was not.

Sentinel replies to SENTINEL CKQUORUM with a status line such as
OK 3 usable Sentinels. Quorum and failover authorization can be reached,
so the bool_ok response callback (== "OK") never matched. When the quorum
cannot be reached, Sentinel replies with an error
(NOQUORUM 1 usable Sentinels. ...), which the parser turned into a plain
ResponseError.

As agreed in #307, the method now returns True when the quorum and the
majority can be reached and False when Sentinel replies NOQUORUM:

• A dedicated parse_sentinel_ckquorum callback accepts any reply that
  starts with OK.
• sentinel_ckquorum() catches ResponseError whose message starts with
  NOQUORUM and returns False. Other response errors (for example
  ERR No such master with that name) are still raised.

The sync and async clients share the callback; the async mixin needs its own
override to await the command before inspecting the error, so
AsyncSentinelCommands.sentinel_ckquorum is added. This applies both when
calling the method on a Valkey client connected to a Sentinel and when
calling it through the Sentinel helper.

Testing

• New unit tests in tests/test_sentinel.py and
  tests/test_asyncio/test_sentinel.py cover the OK ... reply (bytes and
  decoded), the NOQUORUM error, other response errors, and the Sentinel
  helper path.
• Manually verified against the compose.yaml sentinels with both the
  pure-Python and libvalkey parsers, RESP2 and RESP3, sync and async:
  True with all three sentinels up, False after stopping two of them.
• make linters passes locally. make standalone-tests (PROTOCOL=2) passes except for
  the multiprocessing, UDS and network-failure tests, which fail identically on
  unmodified main in my environment (macOS + Docker Desktop).

[mkmkme]

Thanks for your submission, but this is not the right way to fix it I'm afraid

[mkmkme]

Honestly I don't see why this should be its own constant. It's used only once 4 lines below. It should be inlined.

[mkmkme]

This is wrong. This will make the async pipeline commands stop queuing. Previously, this code would immediately add the command to the pipeline:

pipe = client.pipeline(transaction=False)
pipe.sentinel_ckquorum("mymaster")
results = await pipe.execute()

Now it will only create a coroutine. This also breaks the chaining for the pipeline:

pipe.sentinel_ckquorum("master-a").sentinel_ckquorum("master-b")
# AttributeError: 'coroutine' object has no attribute 'sentinel_ckquorum'

[mkmkme]

This will not work in the pipelines. It only works for direct client. Consider this code:

pipe = client.pipeline(transaction=False)
pipe.sentinel_ckquorum("mymaster")  # No response is read here.
results = pipe.execute()           # NOQUORUM is received here.

By the time execute() receives NOQUORUM, sentinel_ckquorum() has already returned. Its exception handler is no longer active.

Instead, pipe.execute() will raise an exception, or return the plain error when raise_on_error=False is passed.

The same applies for async part.