feat(api)!: Support merged listening on multiple tables #5672
Conversation
This file contains 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
jmao-denver
added
DocumentationNeeded
python-server-side
ReleaseNotesNeeded
jmao-denver
changed the title
jmao-denver
changed the title
rcaudy
reviewed
Jul 1, 2024
rcaudy
reviewed
Jul 8, 2024
Integrations/src/main/java/io/deephaven/integrations/python/PythonMergedListenerAdapter.java
Outdated
Show resolved
Hide resolved
chipkent
reviewed
Jul 8, 2024
Integrations/src/main/java/io/deephaven/integrations/python/PythonMergedListenerAdapter.java
Outdated
Show resolved
Hide resolved
jmao-denver
force-pushed
the
5647-merged-listener-wrapper
branch
from
53f8a6c
to
d08a36d
Compare
rcaudy
reviewed
Jul 10, 2024
Integrations/src/main/java/io/deephaven/integrations/python/PythonUtils.java
Outdated
Show resolved
Hide resolved
jmao-denver
force-pushed
the
5647-merged-listener-wrapper
branch
from
92b238a
to
170d9b4
Compare
rcaudy
previously approved these changes
Jul 10, 2024
chipkent
reviewed
Jul 10, 2024
Co-authored-by: Chip Kent <[email protected]>
jmao-denver
force-pushed
the
5647-merged-listener-wrapper
branch
from
b9cf631
to
a153130
Compare
chipkent
previously approved these changes
Jul 12, 2024
rcaudy
requested changes
Jul 12, 2024
engine/table/src/main/java/io/deephaven/engine/table/impl/MergedListener.java
Show resolved
Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonMergedListenerAdapter.java
Show resolved
Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonMergedListenerAdapter.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Ryan Caudy <[email protected]>
rcaudy
reviewed
Jul 13, 2024
rcaudy
reviewed
Jul 15, 2024
chipkent
reviewed
Jul 15, 2024
Co-authored-by: Ryan Caudy <[email protected]>
jmao-denver
changed the title
jmao-denver
changed the title
jmao-denver
changed the title
rcaudy
reviewed
Jul 15, 2024
| @@ -532,6 +492,7 @@ def start(self, do_replay: bool = False) -> None: | |||
| raise RuntimeError("Attempting to start an already started merged listener..") | |||
|
|
|||
| try: | |||
| # self.tables[0] is guaranteed to be a refreshing table | |||
There was a problem hiding this comment.
You could actually make this more elegant and future-proof if you:
- Exposed
is_refreshingandupdate_graphas properties on yourMergedListenerwrapper (MergedListenerHandle). The former is presumably always true, and the latter can delegate to the wrapper Java Object'sgetUpdateGraph(). - Used self as the object to pass to
auto_locking_ctx.
There was a problem hiding this comment.
That's probably more elegant code but I am OK with the current code as it is more 'explicit'.
rcaudy
approved these changes
Jul 15, 2024
chipkent
approved these changes
Jul 15, 2024
|
Labels indicate documentation is required. Issues for documentation have been opened: Community: deephaven/deephaven-docs-community#265 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #5647
BREAKING CHANGE: the replay_lock parameter is removed.