Repository navigation
Fix empty keys in keyspace notifications - #4549
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
petyaslavova
reviewed
Nov 2, 2026
petyaslavova
left a comment
petyaslavova
left a comment
Collaborator
There was a problem hiding this comment.
Hey @BlueX888, thank you for your contribution!
Confirmed: the keyspace and subkeyspace patterns were the only notification forms that dropped empty keys, and the new test covers both. LGTM once CI is green.
petyaslavova
approved these changes
Nov 2, 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
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.
Problem
KeyNotification.try_parsedrops keyspace notifications for empty keys: the_KEYSPACE_PATTERNand_SUBKEYSPACE_PATTERNregexes use(.+)for the key, so a channel like__keyspace@0__:— a zero-length key, which is valid in Redis (e.g.SET "" value) — fails to parse andtry_parsereturnsNone.Change
Allow an empty key in the two keyspace patterns (
(.+)->(.*)). The keyevent patterns keep(.+): Redis event names are never empty. Nothing else in the parsing changes.Testing
test_empty_keyspace_key(str and bytes, keyspace and subkeyspace channels): an empty key parses,key == "", subkeys andis_keyspaceas expected.tests/test_keyspace_notifications.pypass, including the trailing-newline and key-prefix cases there.DOTALLcomment guards.fixed_client,standalone,cluster) run locally against Redis 8.10.3 with zero candidate-caused failures — verified against an identical base run in the same window, with the handful of differing failures re-confirmed as timing-flaky on both sides.Note
Low Risk
Narrow regex change in notification parsing with targeted regression tests; no auth, I/O, or broader API behavior changes.
Overview
KeyNotification.try_parsepreviously returnedNonefor channels whose Redis key is empty (e.g.__keyspace@0__:afterSET "" value), because_KEYSPACE_PATTERNand_SUBKEYSPACE_PATTERNrequired at least one character with(.+).Those two patterns now use
(.*)so a zero-length key parses askey == "". Keyevent-style patterns are unchanged ((.+)), since event names are never empty.Adds parametrized
test_empty_keyspace_key(str/bytes, keyspace and subkeyspace) to lock in empty-key parsing and subkey handling.Reviewed by Cursor Bugbot for commit 59c0d6c. Bugbot is set up for automated code reviews on this repo. Configure here.