Skip to content
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

fix: Don't memoize retrieval of tables from JsPartitionedTable#getTable #5009

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Jan 5, 2024

- Memoizing it is inconsistent with other methods like `WorkerConnection.getTable`, which always returns a new table
- Was unclear who the "owner" of the returned table was
- Memoization can happen at the client level if they need it
@mofojed mofojed added NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed. labels Jan 5, 2024
@mofojed mofojed requested a review from niloc132 January 5, 2024 19:24
@mofojed mofojed self-assigned this Jan 5, 2024
- Ensures that the values have not mutated when executing the lambda
@mofojed mofojed requested a review from niloc132 January 12, 2024 18:37
@mofojed mofojed enabled auto-merge (squash) January 12, 2024 18:41
@mofojed mofojed merged commit 1e0525e into deephaven:main Jan 12, 2024
18 checks passed
@mofojed mofojed deleted the partitioned-table-gettable branch January 12, 2024 18:52
@github-actions github-actions bot locked and limited conversation to collaborators Jan 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants