Adding OSS Notification Classes for SMIGRATING and SMIGRATED. Handling of SMIGRATING is completed and covered with tests. #3849
Conversation
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
petyaslavova force-pushed the ps_add_cluster_related_notifications_and_handlers branch from b7bf8e3 to a983c20 Compare …ceholder for smigrated handler in OSSMaintNotificationsHandler class
petyaslavova force-pushed the ps_add_cluster_related_notifications_and_handlers branch from a983c20 to e3c8b3f Compare petyaslavova changed the title Copilot finished reviewing on behalf of petyaslavova November 19, 2025 09:33
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces OSS (Open Source Software) cluster maintenance notification support for handling slot migration events in Redis. The implementation adds two new notification classes (OSSNodeMigratingNotification and OSSNodeMigratedNotification) along with parser integration and comprehensive unit tests for the notification classes themselves.
- New notification classes to track slot migration with source/destination nodes and affected slots
- Thread-safe
OSSMaintNotificationsHandlerfor managing maintenance notifications with expiration cleanup - Parser integration to recognize and route OSS maintenance notifications (SMIGRATING and SMIGRATED messages)
- Full unit test coverage for notification class behavior (initialization, equality, hashing, collections)
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| redis/maint_notifications.py | Adds OSSNodeMigratingNotification, OSSNodeMigratedNotification classes and OSSMaintNotificationsHandler for managing OSS slot migration notifications |
| redis/_parsers/base.py | Adds parsing logic for SMIGRATING/SMIGRATED messages and integrates OSS notification handlers into push notification routing |
| redis/_parsers/resp3.py | Adds oss_cluster_maint_push_handler_func attribute to support OSS maintenance notification handling |
| redis/_parsers/hiredis.py | Adds oss_cluster_maint_push_handler_func attribute to support OSS maintenance notification handling |
| tests/maint_notifications/test_maint_notifications.py | Comprehensive unit tests for OSSNodeMigratingNotification and OSSNodeMigratedNotification classes |
| tests/maint_notifications/test_cluster_maint_notifications_handling.py | Integration test for SMIGRATING notification handling and test infrastructure refactoring |
| tests/maint_notifications/proxy_server_helpers.py | Helper functions for RESP format conversion and HTTP client response handling improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/maint_notifications/test_cluster_maint_notifications_handling.py Outdated Show resolved Hide resolved
tests/maint_notifications/test_cluster_maint_notifications_handling.py Outdated Show resolved Hide resolved
vladvildanov approved these changes Nov 19, 2025
petyaslavova merged commit a623b2e into feat/hitless_upgrade_sync_cluster_client 4 checks passed
petyaslavova deleted the ps_add_cluster_related_notifications_and_handlers branch petyaslavova added a commit that referenced this pull request Nov 20, 2025
…g of SMIGRATING is completed and covered with tests. (#3849) * Adding maintenance notifications for OSS API enabled connections * Adding oss maint notifications handler configurations to parsers. Placeholder for smigrated handler in OSSMaintNotificationsHandler class * Applying review comments. Finilized the notifications format.
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.
In this PR the following changes are introduced:
OSS Notification Classes: New
OSSNodeMigratingNotificationandOSSNodeMigratedNotificationclasses that track slot migration events with source/destination nodes and affected slotsThread-Safe Handler:
OSSMaintNotificationsHandlerprovides thread-safe management of maintenance notifications with automatic expiration cleanupParser Integration: Seamless integration with existing parser infrastructure to recognize and route OSS maintenance notifications
Testing: Full unit test coverage for all notification classes ensuring proper behavior across initialization, equality, hashing, and collection operations
Out of scope for the current PR: initializing the nodes manager handler that will react on SMIGRATED notifications.