Skip to content

Better stream table support over Barrage and jsapi#2475

Merged
nbauernfeind merged 7 commits intodeephaven:mainfrom
nbauernfeind:barrage_stream
Jun 22, 2022
Merged

Better stream table support over Barrage and jsapi#2475
nbauernfeind merged 7 commits intodeephaven:mainfrom
nbauernfeind:barrage_stream

Conversation

@nbauernfeind
Copy link
Copy Markdown
Contributor

@nbauernfeind nbauernfeind commented Jun 3, 2022

Fixes #2377. Fixes #1492.

It also adds some stream table specific changes to jsTable that prevent flickering and improve overall GUI experience for stream tables.

@nbauernfeind nbauernfeind added this to the Jun 2022 milestone Jun 3, 2022
@nbauernfeind nbauernfeind requested review from lbooker42 and rcaudy June 3, 2022 22:02
@nbauernfeind nbauernfeind self-assigned this Jun 3, 2022
@nbauernfeind nbauernfeind changed the title Better stream table support over Barrage in jsapi Better stream table support over Barrage and jsapi Jun 3, 2022
Copy link
Copy Markdown
Contributor

@lbooker42 lbooker42 left a comment

Choose a reason for hiding this comment

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

These changes look good and are easy to follow, would like to see unit tests exercising this code in BarrageMessageProducer and the associated BarrageTables . The web changes should be reviewed by @niloc132.

@nbauernfeind nbauernfeind requested review from niloc132 and removed request for lbooker42 June 7, 2022 14:37
Comment thread web/client-api/src/main/java/io/deephaven/web/client/api/JsTable.java Outdated
Comment thread web/client-api/src/main/java/io/deephaven/web/client/api/JsTable.java Outdated
Comment thread web/client-api/src/main/java/io/deephaven/web/client/api/JsTable.java Outdated
Comment thread web/client-api/src/main/java/io/deephaven/web/client/api/JsTable.java Outdated
Comment thread server/src/main/java/io/deephaven/server/barrage/BarrageMessageProducer.java Outdated
niloc132
niloc132 previously approved these changes Jun 13, 2022
Copy link
Copy Markdown
Member

@niloc132 niloc132 left a comment

Choose a reason for hiding this comment

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

Marked approved, but with one "maybe change this, it seems potentially better to me" and one "this could be simpler if you wanted", but both optional.

Comment thread web/client-api/src/main/java/io/deephaven/web/client/api/JsTable.java Outdated
@nbauernfeind nbauernfeind merged commit a1a1464 into deephaven:main Jun 22, 2022
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve stream semantics over barrage stream table semantics via Barrage

3 participants