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

Async Table and View Constructor #1289

Merged
merged 2 commits into from
Feb 12, 2021
Merged

Async Table and View Constructor #1289

merged 2 commits into from
Feb 12, 2021

Conversation

sc1f
Copy link
Contributor

@sc1f sc1f commented Jan 12, 2021

This PR makes a breaking public API change:

The table() and view() constructors in Javascript and the Python Tornado client are now async/await. In Javascript, perspective.table() and table.view() now return Promises that will resolve to a Table or View, or be rejected with an error. This allows for much clearer error handling during table and view construction, especially in the client/server model where it is important for the client to know why a table/view could not be constructed and to act on that error information. Additionally, this enables better error reporting from perspective-viewer itself, as the viewer can load() a Promise() => Table and display the error if the promise is rejected.

Additionally, this PR also improves the error reporting of the WASM binding. Previously, an error message was logged to console.warning and an Error("Abort()" was thrown:

Screen Shot 2021-01-19 at 2 25 28 PM

This PR simplifies error reporting by throwing an Error with the custom error message that was previously just logged, which should simplify the way errors are thrown and make the error condition more clear:

Screen Shot 2021-01-19 at 2 25 46 PM

Changelog

  • async Table and View constructors in JS, and Python Tornado client
  • Refactored perspective-viewer, perspective-viewer-jupyterlab to use async/await constructors
  • Refactored docs, all examples, all tests
  • Added error handling & load(Promise) tests
    Running benchmarks on this PR, 0.6.0, and 0.5.6 shows that performance is not impacted by this API change:

Screen Shot 2021-01-12 at 5 53 58 PM

@sc1f sc1f added enhancement Feature requests or improvements JS Python WIP breaking labels Jan 12, 2021
@sc1f sc1f self-assigned this Jan 12, 2021
@sc1f sc1f changed the title Async Table and View constructors Async Table and View constructor Jan 12, 2021
@sc1f sc1f marked this pull request as draft January 13, 2021 19:19
@sc1f sc1f force-pushed the async-table-view branch 3 times, most recently from 5721e8a to 14a5e11 Compare January 19, 2021 16:36
@texodus texodus added the 0.7.0 label Jan 19, 2021
@sc1f sc1f changed the title Async Table and View constructor Async Table and View Constructor Jan 19, 2021
@sc1f sc1f force-pushed the async-table-view branch 2 times, most recently from 60c9c1e to 3e3ad08 Compare January 19, 2021 21:56
@texodus texodus removed the WIP label Jan 19, 2021
@sc1f sc1f marked this pull request as ready for review January 21, 2021 19:54
@texodus texodus removed the 0.7.0 label Jan 30, 2021
@texodus texodus added this to the 0.7.0 milestone Jan 30, 2021
sc1f and others added 2 commits February 11, 2021 16:26
WIP: await table.view()

clean up server

Fix rest of viewer, refactor perspective/api/server

Fix examples, JS tests, jupyterlab to use async view constructor

Fix examples and JS core tests, TODO: fix websocket tests

Node server returns promise to view

Fix filter test for computed

Fix jupyterlab - use async view

Async table constructor

Fix examples and docs

Fix tests, jupyterlab for async table constructor

Fix engine tests

Fix jupyterlab for remote table

Fix jlab build - use fat build

WIP: fix workspace tests

wait_for_update must be true otherwise the screenshot happens before table is loaded async

refactor jupyter widget

rebase on master, fix docs, tests, benchmark

rebase on master, fix docs, tests, benchmark suite

fix jlab tests

update jlab, webpack plugin to 0.6.0, remove test.run

update jlab, webpack plugin to perspective 0.6.0

remove test.run

async table/view on tornado client

fix all tests

Errors thrown with verbose message from WASM, remove closure compiler

assert that we can load a promise

Skip flaky d3fc tests

rebase master
Copy link
Member

@texodus texodus left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the PR!

I fixed up the regressions since 0.6.0, and reverted the test changes (these are mostly fixed on master anyway and I'm not sure this PR's test fixes do ..)

@texodus texodus merged commit 44029bb into master Feb 12, 2021
@texodus texodus deleted the async-table-view branch February 12, 2021 04:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking enhancement Feature requests or improvements JS Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants