-
Notifications
You must be signed in to change notification settings - Fork 179
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
feat: Add a daft dashboard to display queries plans and stats #3790
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #3790 will improve performances by 11.82%Comparing Summary
Benchmarks breakdown
|
- this is so that from Python, we can serve the embedded static dashboard assets from any virtual env
daft/static_dashboard_assets/_next/static/chunks/1183.aecd3c90e74b398d.js
Dismissed
Show dismissed
Hide dismissed
daft/static_dashboard_assets/_next/static/chunks/1183.aecd3c90e74b398d.js
Dismissed
Show dismissed
Hide dismissed
daft/static_dashboard_assets/_next/static/chunks/2124.698e7011cf2dd6cf.js
Dismissed
Show dismissed
Hide dismissed
daft/static_dashboard_assets/_next/static/chunks/2124.698e7011cf2dd6cf.js
Dismissed
Show dismissed
Hide dismissed
daft/static_dashboard_assets/_next/static/chunks/5341.e269c7efc9743966.js
Dismissed
Show dismissed
Hide dismissed
daft/static_dashboard_assets/_next/static/chunks/5341.e269c7efc9743966.js
Dismissed
Show dismissed
Hide dismissed
daft/static_dashboard_assets/_next/static/chunks/7715.2dfa101bbf9b21e6.js
Dismissed
Show dismissed
Hide dismissed
daft/static_dashboard_assets/_next/static/chunks/7715.2dfa101bbf9b21e6.js
Dismissed
Show dismissed
Hide dismissed
daft/static_dashboard_assets/_next/static/chunks/7715.2dfa101bbf9b21e6.js
Dismissed
Show dismissed
Hide dismissed
daft/static_dashboard_assets/_next/static/chunks/7715.2dfa101bbf9b21e6.js
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a high level:
-
Why is everything in
src/
? Previously we had stuff indashboard/
which made sense because the server and typescript code doesn't need to live inside of Daft itself. -
How much does this increase the binary size of the Daft wheel by, since we're packaging all this stuff now with the Daft wheel I'm assuming? Do we need to add a new
getdaft[dashboard]
perhaps?
I think to merge this we should think a little harder about the packaging story. Seems like things are all shoved into the main Daft binary today which isn't ideal. Can we lay out how things are packaged in the PR description?
@@ -144,11 +144,12 @@ dependencies = [ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is Cargo lock changing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The src/daft-dashboard-server
uses hyper
and some other http-frameworks to:
- Serve the daft broadcast listener
- Serve the dashboard API
- Serve the dashboard web server
@@ -158,6 +160,45 @@ def _result(self) -> Optional[PartitionSet]: | |||
else: | |||
return self._result_cache.value | |||
|
|||
def _explain_broadcast(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like dashboard-specific code. Let's define this outside of dataframe.py
if possible, likely some kind of Client
exposed by the dashboard module I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can update it to live inside of the daft/dashboard.py
file.
Overview
This PR introduces a new (statically built and served) dashboard web-application UI.
We have a simple server that binds to port 3238 and does 3 things:
The static HTML files which are served can then be accessed by pointing your browser towards
http::localhost:3238
.Usage
Notes
The server process is launched off as an orphaned process. It will live on even after the script that initialized it has completed.
You can kill this process (only manually as of now) by running
kill -9 $(lsof -t -i :3238)
.Note to reviewers
Although this PR claims to be a big diff, in reality, it's only big because of a lot of generated React UI components (inside of
src/daft-dashboard-client
).For backend reviewers, please review
src/daft-dashboard-server
anddaft
. For frontend reviewers, please reviewsrc/daft-dashboard-client
.Generated files
Please ignore all the generated HTML/CSS/JS files inside of
daft/static_dashboard_assets
. They are automatically generated bybun run build
during prior to release.These files have to be checked into the repo since they need to be released alongside daft wheels.