fix(messaging): Azure receive() could never decode a body; Redis resilience for managed cache - #18
Merged
abhi-bhat-lyzr merged 4 commits intoJul 29, 2026
Conversation
Every Redis factory built `redis.asyncio.Redis` with library defaults: `health_check_interval=0` and `Retry(NoBackoff(), 0)` — no health checks and zero retries. Managed Redis (Azure Cache for Redis, ElastiCache) reaps idle connections and drops every connection during maintenance, failover, and scale, so the first command after the server hangs up raised `ConnectionError: Connection closed by server.` at the caller. This took down lyzr-memory in production: every authenticated request 500'd. Adds `resilient_client_kwargs()` — retry with exponential backoff (0.1/0.2/0.4s) on ConnectionError and TimeoutError, a 30s health-check interval, TCP keepalive, 5s socket timeouts, and a 100-connection ceiling — and applies it in every factory on all three backends. Each factory takes **client_kwargs so a call site can override any of it. Tests assert the settings reach both the pool kwargs and the Connection object built from them, that overrides win, and — by signature — that no factory can be added without the passthrough.
…he caller Azure Cache for Redis promotes the replica on failover and during maintenance. A pooled connection can still be pointed at the node that just became a replica, which answers writes with `-READONLY You can't write against a read only replica.` redis-py raises that as ReadOnlyError, a ResponseError subclass — not a ConnectionError — so the retry_on_error list added in the previous commit did not cover it and the error surfaced to the caller. Retrying disconnects and reconnects, which re-resolves DNS to the newly promoted primary. Removing ReadOnlyError from the list fails 6 tests.
The live Azure deployment is Azure Managed Redis (<name>.<region>.redis.azure.net:10000), which is TLS-only and ACL-based. from_credentials defaulted ssl=False and had no way to pass a certificate policy, so a caller could only produce a plaintext client. Connecting in plaintext to a TLS-only port gets the connection closed by the server and surfaces as `ConnectionError: Connection closed by server.` on every command — verified against the live cache: plaintext fails, TLS pings. - from_credentials: add ssl_cert_reqs. - from_access_key: add username and ssl_cert_reqs, and document that the two Azure Redis products differ in host, port and auth (Cache for Redis is password-only on 6380; Managed Redis is ACL-based on 10000). ssl_cert_reqs defaults to "required", not None: redis-py silently resolves None to CERT_NONE, so a None default would turn off certificate verification for every TLS caller without saying so.
receive() built its Message with bytes(m), but ServiceBusReceivedMessage does
not implement __bytes__ — its .body is a single-use generator of chunks. Every
receive against a live broker raised:
TypeError: cannot convert 'ServiceBusReceivedMessage' object to bytes
so the Azure consume path had never worked. Verified against Service Bus
namespace lyzr-studio-dev-we-sbus: type(m.body) is 'generator' and
b"".join(m.body) yields the payload.
The fake in tests/test_messaging_azure.py defined __bytes__, which the real
type lacks, so bytes(m) passed in CI and failed on first contact with a broker.
The fake now mirrors the SDK: .body yields chunks and is single-use. Restoring
bytes(m) with the corrected fake fails 4 tests with the production error.
Bumps to 0.2.8.
|
abhi-bhat-lyzr
approved these changes
Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Azure Service Bus
receive()has never workedreceive()built itsMessagewithbytes(m), butServiceBusReceivedMessagedoes not implement__bytes__— its.bodyis a single-use generator of byte chunks. Every receive against a live broker raised:Sending was fine, so the break only shows up the moment anything tries to consume. Present in 0.2.6 and 0.2.7 alike.
Why CI never caught it: the fake in
tests/test_messaging_azure.pydefined__bytes__, which the real SDK type lacks.bytes(m)passed against the fake and failed against the broker. The fake now mirrors the SDK —.bodyyields chunks and is single-use — so restoringbytes(m)fails 4 tests with the production error.Verified against namespace
lyzr-studio-dev-we-sbus:type(m.body)isgenerator,b"".join(m.body)yields the payload, and a full send → receive → complete round trip now succeeds on a session-enabled queue.Redis resilience for managed cache
Three earlier commits in this branch:
ReadOnlyErrorwas reaching the caller. It subclassesResponseError, notConnectionError, so Azure's failover reply (-READONLY You can't write against a read only replica.) skipped the retry path. Retrying reconnects and re-resolves DNS to the promoted primary.from_credentials/from_access_keyacceptusernameandssl_cert_reqs.ssl_cert_reqsdefaults to"required"; redis-py silently resolvesNonetoCERT_NONE, which disables verification without saying so.Verification
267 passed, 3 skipped. The one failure,test_azure_credentials.py::test_crypto_backend_uses_the_shared_chain, is pre-existing and unrelated (No module named 'azure.keyvault').Version bumped to 0.2.8.