Reorganizing the code related to maintenance push notifications. #3785
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR reorganizes the maintenance push notifications code by extracting logic into separate abstract classes and using multiple inheritance to expose maintenance push notifications for existing Connection and ConnectionPool classes.
- Introduced abstract classes for maintenance push notifications functionality
- Updated connection handling to use the new
_get_socket()method instead of direct_sockaccess - Modified test configurations to support cache configurations and different connection types
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_maint_notifications_handling.py | Updated test methods to use new socket access patterns and added cache configuration support |
| tests/test_maint_notifications.py | Added socket import and updated MockConnection to inherit from new abstract class |
| redis/maint_notifications.py | Updated type hints to use new MaintNotificationsAbstractConnection instead of ConnectionInterface |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
petyaslavova
force-pushed
the
ps_extract_hitless_related_connection_management_logic_in_mixin_classes
branch
from
feeed3f to
97bd380
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
petyaslavova
force-pushed
the
ps_extract_hitless_related_connection_management_logic_in_mixin_classes
branch
from
c30f625 to
7f99734
Compare
…classes. Usingle multiple inheritance for Connection and ConnectionPool classes to expose the maintenance push notifications for the existing connections.
…face as it is also used by AA
petyaslavova
force-pushed
the
ps_extract_hitless_related_connection_management_logic_in_mixin_classes
branch
from
f17b1f9 to
c75397e
Compare
| if isinstance(self._conn, MaintNotificationsAbstractConnection): | ||
| return self._conn.maintenance_state | ||
| else: | ||
| raise NotImplementedError( |
There was a problem hiding this comment.
Let's extract this part as it used in multiple places
There was a problem hiding this comment.
Done. Extracted those in a helper method that will raise the error if the connection is not a MaintNotificationsAbstractConnection and will return the provided object in case it is - I'm doing this so that I can somehow "show" the static code analysis tools that this object is an instance of the correct type and those attributes are available and errors should not be reported.
|
|
||
| def get_endpoint_type( | ||
| self, host: str, connection: "ConnectionInterface" | ||
| self, host: str, connection: "MaintNotificationsAbstractConnection" |
There was a problem hiding this comment.
Needs to make sure to release as a part of 7.0.0, otherwise breaking change
There was a problem hiding this comment.
Yeah, I know - that's why I've been working on this with the highest priority the last days :)
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
Extracting maintenance push notifications logic into separate abstract classes. Using multiple inheritance for
ConnectionandConnectionPoolclasses to expose the maintenance push notifications for the existing connections.