nested refresh#102
Conversation
There was a problem hiding this comment.
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 |
| _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 |
There was a problem hiding this comment.
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( |
There was a problem hiding this comment.
[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.
| ZnSocketObject, | ||
| ) | ||
| from znsocket.client import Client | ||
| from znsocket.client import Client, _get_clients_with_nested_refresh |
There was a problem hiding this comment.
[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.