-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Adding 'auto' option to MaintNotificationsConfig.enabled #3779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding 'auto' option to MaintNotificationsConfig.enabled #3779
Conversation
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new "auto" option to the MaintNotificationsConfig.enabled
parameter, expanding the configuration from a simple boolean to support three modes: disabled (False), enabled (True), and auto ("auto"). The auto mode allows the client to attempt maintenance notifications but gracefully continue if the server doesn't support them, rather than failing the connection.
Key changes:
- Updated the
enabled
parameter type to accept boolean or "auto" string literal - Modified connection handshake logic to handle auto mode with graceful fallback
- Added comprehensive test coverage for the new handshake behaviors
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
redis/maint_notifications.py |
Updates MaintNotificationsConfig.enabled type and default value to support "auto" mode |
redis/connection.py |
Implements auto mode logic in handshake with graceful error handling for unsupported servers |
tests/test_maint_notifications.py |
Updates default value test to reflect new "auto" default |
tests/test_maint_notifications_handling.py |
Adds comprehensive test coverage for handshake scenarios and refactors test structure |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
disabled: The client doesn’t send the CLIENT MAINT_NOTIFICATIONS ON ... command as part of the handshake. The feature isn’t used by the client.
enabled: The client forcefully sends the CLIENT MAINT_NOTIFICATIONS ON ... command to the server. If the server responds with an error, the connection is interrupted and an error is raised.
auto: The client tries to send the CLIENT MAINT_NOTIFICATIONS ON ... command to the server. If the server returns an error, the client should continue without the push notifications.