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: Expose base, "underlying" table for Partitioned Table Enhancements #5645

Merged
merged 11 commits into from
Jul 3, 2024

Conversation

AkshatJawne
Copy link
Contributor

@AkshatJawne AkshatJawne commented Jun 21, 2024

Closes deephaven/web-client-ui#2079 (All the core changes, web-client-ui changes to come in corresponding PR)

Changes Implemented

  • Create a baseTable member variable and have a corresponding getter method, in order to be able to expose it for Partitioned Tables, as desired in point 1 of this epic

@AkshatJawne AkshatJawne self-assigned this Jun 21, 2024
@AkshatJawne
Copy link
Contributor Author

Applying spotlessApply to resolve Quick CI fix, then repushing -- taking long to start Gradle Daemon

@AkshatJawne AkshatJawne requested a review from mofojed June 24, 2024 15:28
@AkshatJawne
Copy link
Contributor Author

Ready for re-review

@AkshatJawne AkshatJawne requested a review from mofojed June 24, 2024 20:37
@mofojed mofojed changed the title fix: Expose base, "underlying "table for Partitioned Table Enhancements fix: Expose base, "underlying" table for Partitioned Table Enhancements Jun 25, 2024
Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussing with @niloc132 a bit, there's a couple of changes we want to make.

  • Main change - we want to deprecate the getKeyTable method, and instead the UI should be able to get the keys from the baseTable
    • Limit viewport to the key columns that matter when setting the viewport
    • Means we don't create and subscribe to two tables unnecessarily, better performance
    • To deprecate the method, we still need it to work - but instead of assigning keys table in the refetch method, we want to lazily fetch it when the getKeyTable() method is called. Then UI has a bit of grace period before it needs to update

@AkshatJawne
Copy link
Contributor Author

Will make changes here (getKeyTable deprecation), changes to have view on keys table in : #5669

@AkshatJawne AkshatJawne requested a review from mofojed June 28, 2024 14:04
@AkshatJawne
Copy link
Contributor Author

Looking into failures here

@AkshatJawne AkshatJawne requested a review from niloc132 July 3, 2024 19:43
@AkshatJawne AkshatJawne merged commit bb810e4 into deephaven:main Jul 3, 2024
16 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 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.

EPIC: Partitioned Table UI improvements
3 participants