-
Notifications
You must be signed in to change notification settings - Fork 2
nested refresh #102
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
base: main
Are you sure you want to change the base?
nested refresh #102
Conversation
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
Adds support for nested refresh callbacks so that parent List/Dict objects receive refresh events when their nested children change. Key changes include:
- Tracking parent watchers and propagating refresh notifications from child objects
- Extending
on_refreshAPI to enable nested refresh and registering client instances globally - Adding tests for nested refresh behavior in both List and Dict
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| znsocket/objects/list_obj.py | Track parent watchers, notify on nested changes, update on_refresh API |
| znsocket/objects/dict_obj.py | Same nested watcher/notification logic for Dict |
| znsocket/client.py | Global client registry and helper to find nested-refresh-enabled clients |
| tests/test_list.py | New test_list_nested_refresh scenario |
| tests/test_dict.py | New test_dict_nested_refresh scenario |
znsocket/client.py
Outdated
| _all_clients: list[weakref.ref] = [] | ||
|
|
||
|
|
||
| def _get_clients_with_nested_refresh(key: str) -> list["Client"]: | ||
| """Get all client instances that have nested refresh enabled for the given key.""" | ||
| clients = [] | ||
| # Clean up dead references | ||
| global _all_clients | ||
| _all_clients = [ref for ref in _all_clients if ref() is not None] | ||
|
|
||
| for client_ref in _all_clients: | ||
| client = client_ref() | ||
| if ( | ||
| client | ||
| and key in client.nested_refresh_keys |
Copilot
AI
Jul 15, 2025
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.
Use a weakref.WeakSet to track client instances instead of a list of weakrefs and manual filtering; this simplifies cleanup and improves performance.
| _all_clients: list[weakref.ref] = [] | |
| def _get_clients_with_nested_refresh(key: str) -> list["Client"]: | |
| """Get all client instances that have nested refresh enabled for the given key.""" | |
| clients = [] | |
| # Clean up dead references | |
| global _all_clients | |
| _all_clients = [ref for ref in _all_clients if ref() is not None] | |
| for client_ref in _all_clients: | |
| client = client_ref() | |
| if ( | |
| client | |
| and key in client.nested_refresh_keys | |
| _all_clients: weakref.WeakSet["Client"] = weakref.WeakSet() | |
| def _get_clients_with_nested_refresh(key: str) -> list["Client"]: | |
| """Get all client instances that have nested refresh enabled for the given key.""" | |
| clients = [] | |
| for client in _all_clients: | |
| if ( | |
| key in client.nested_refresh_keys |
| """Whether nested refresh is enabled for this object.""" | ||
| return self._nested_refresh | ||
|
|
||
| def _add_parent_watcher( |
Copilot
AI
Jul 15, 2025
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.
[nitpick] The nested-refresh watcher methods are duplicated in both List and Dict; consider extracting this logic into a shared base class or mixin to reduce duplication.
znsocket/objects/list_obj.py
Outdated
| ZnSocketObject, | ||
| ) | ||
| from znsocket.client import Client | ||
| from znsocket.client import Client, _get_clients_with_nested_refresh |
Copilot
AI
Jul 15, 2025
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.
[nitpick] Importing a private helper (_get_clients_with_nested_refresh) may break encapsulation; consider making this function part of the public API or renaming it.
| from znsocket.client import Client, _get_clients_with_nested_refresh | |
| from znsocket.client import Client, get_clients_with_nested_refresh |
Pull Request Test Coverage Report for Build 16286457857Details
💛 - Coveralls |
Uh oh!
There was an error while loading. Please reload this page.