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.

Sign up for GitHub

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

Jump to bottom

Allow bokeh=3 #5648

Merged
merged 20 commits into from
Nov 15, 2022
Merged

Allow bokeh=3 #5648

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3d192fb
Use development version of bokeh in CI
jrbourbeau Jan 7, 2022
5e9b9b3
Merge branch 'main' of https://github.com/dask/distributed into dev-b…
jrbourbeau Jan 10, 2022
6436877
Merge branch 'main' of https://github.com/dask/distributed into dev-b…
jrbourbeau Oct 21, 2022
45477f7
More versions
jrbourbeau Oct 21, 2022
46dbafe
Panel / TabPanel compat
jrbourbeau Oct 21, 2022
1db6c2f
Make GHA yaml valid?
jrbourbeau Oct 21, 2022
7b1056c
id and Div compat
jrbourbeau Oct 24, 2022
0811fec
CDSView filter
jrbourbeau Oct 24, 2022
8d8f5f3
Div style
jrbourbeau Oct 24, 2022
b98f706
Lingering id
jrbourbeau Oct 24, 2022
3e51931
Remove ids
jrbourbeau Oct 26, 2022
39d3a89
Handle DataTable CSS
jrbourbeau Oct 27, 2022
7192b2b
Merge branch 'main' of https://github.com/dask/distributed into dev-b…
jrbourbeau Oct 28, 2022
97fc007
Merge branch 'main' of https://github.com/dask/distributed into dev-b…
jrbourbeau Nov 8, 2022
dee0277
Install old bokeh in CI to check backwards compatibility
jrbourbeau Nov 8, 2022
7c9477b
Remove bokeh version upper bound
jrbourbeau Nov 8, 2022
3bc45cc
Update env file
jrbourbeau Nov 14, 2022
6dd05a4
Try importlib.metadata workaround for distributed/tests/test_init.py
jrbourbeau Nov 14, 2022
be63adb
Try bokeh=3.0.2
jrbourbeau Nov 14, 2022
b38f6d9
Remove temporary pip install
jrbourbeau Nov 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@ jobs:
run: mamba env update -n dask-distributed -f ${{ env.CONDA_FILE }}
if: steps.skip-caching.outputs.trigger-found == 'true' || steps.cache.outputs.cache-hit != 'true'

- name: Install dev bokeh
if: matrix.python-version == '3.9'
shell: bash -l {0}
run: |
conda uninstall --force bokeh
mamba install -c bokeh/label/dev bokeh

- name: Install
shell: bash -l {0}
run: |
Expand Down
83 changes: 42 additions & 41 deletions distributed/dashboard/components/scheduler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
NumberFormatter,
NumeralTickFormatter,
OpenURL,
Panel,
PanTool,
Range1d,
ResetTool,
Expand Down Expand Up @@ -72,6 +71,7 @@
ProfileTimePlot,
SystemMonitor,
)
from distributed.dashboard.core import TabPanel
from distributed.dashboard.utils import BOKEH_VERSION, PROFILING, transpose, update
from distributed.diagnostics.graph_layout import GraphLayout
from distributed.diagnostics.progress import GroupTiming
Expand Down Expand Up @@ -134,7 +134,7 @@ def __init__(self, scheduler, **kwargs):
title="Occupancy",
tools="",
toolbar_location="above",
id="bk-occupancy-plot",
# id="bk-occupancy-plot",
x_axis_type="datetime",
min_border_bottom=50,
**kwargs,
Expand Down Expand Up @@ -211,7 +211,7 @@ def __init__(self, scheduler, **kwargs):

self.root = figure(
title="Tasks Processing (count)",
id="bk-nprocessing-histogram-plot",
# id="bk-nprocessing-histogram-plot",
name="processing",
y_axis_label="frequency",
tools="",
Expand Down Expand Up @@ -316,7 +316,7 @@ def __init__(self, scheduler, width=600, **kwargs):
self.root = figure(
title="Bytes stored on cluster",
tools="",
id="bk-cluster-memory-plot",
# id="bk-cluster-memory-plot",
width=int(width / 2),
name="cluster_memory",
min_border_bottom=50,
Expand Down Expand Up @@ -454,7 +454,7 @@ def __init__(self, scheduler, width=600, **kwargs):
self.root = figure(
title="Bytes stored per worker",
tools="",
id="bk-workers-memory-plot",
# id="bk-workers-memory-plot",
width=int(width / 2),
name="workers_memory",
min_border_bottom=50,
Expand Down Expand Up @@ -595,7 +595,7 @@ def __init__(self, scheduler, **kwargs):
self.root = figure(
title="Bytes stored per worker",
name="workers_memory",
id="bk-workers-memory-histogram-plot",
# id="bk-workers-memory-histogram-plot",
y_axis_label="frequency",
tools="",
**kwargs,
Expand Down Expand Up @@ -650,7 +650,7 @@ def __init__(self, scheduler, width=600, **kwargs):
self.root = figure(
title=f"Bytes transferring: {format_bytes(0)}",
tools="",
id="bk-workers-transfer-bytes-plot",
# id="bk-workers-transfer-bytes-plot",
width=int(width / 2),
name="workers_transfer_bytes",
min_border_bottom=50,
Expand Down Expand Up @@ -902,7 +902,7 @@ def __init__(self, scheduler, **kwargs):
self.root = figure(
title="Bandwidth by Type",
tools="",
id="bk-bandwidth-type-plot",
# id="bk-bandwidth-type-plot",
name="bandwidth_type_histogram",
y_range=["a", "b"],
**kwargs,
Expand Down Expand Up @@ -973,7 +973,7 @@ def __init__(self, scheduler, **kwargs):
self.root = figure(
title="Bandwidth by Worker",
tools="",
id="bk-bandwidth-worker-plot",
# id="bk-bandwidth-worker-plot",
name="bandwidth_worker_heatmap",
x_range=["a", "b"],
y_range=["a", "b"],
Expand Down Expand Up @@ -1071,7 +1071,7 @@ def __init__(self, scheduler, **kwargs):
self.bandwidth = figure(
title="Worker Network Bandwidth",
tools="",
id="bk-worker-net-bandwidth",
# id="bk-worker-net-bandwidth",
name="worker_network_bandwidth",
**kwargs,
)
Expand Down Expand Up @@ -1112,7 +1112,7 @@ def __init__(self, scheduler, **kwargs):
self.disk = figure(
title="Workers Disk",
tools="",
id="bk-workers-disk",
# id="bk-workers-disk",
name="worker_disk",
**kwargs,
)
Expand Down Expand Up @@ -1245,7 +1245,7 @@ def __init__(self, scheduler, follow_interval=20000, **kwargs):
x_axis_type="datetime",
tools=tools,
x_range=x_range,
id="bk-worker-network-bandwidth-ts",
# id="bk-worker-network-bandwidth-ts",
name="worker_network_bandwidth-timeseries",
**kwargs,
)
Expand Down Expand Up @@ -1277,7 +1277,7 @@ def __init__(self, scheduler, follow_interval=20000, **kwargs):
x_axis_type="datetime",
tools=tools,
x_range=x_range,
id="bk-worker-cpu-ts",
# id="bk-worker-cpu-ts",
name="worker_cpu-timeseries",
**kwargs,
)
Expand All @@ -1297,7 +1297,7 @@ def __init__(self, scheduler, follow_interval=20000, **kwargs):
x_axis_type="datetime",
tools=tools,
x_range=x_range,
id="bk-worker-memory-ts",
# id="bk-worker-memory-ts",
name="worker_memory-timeseries",
**kwargs,
)
Expand All @@ -1318,7 +1318,7 @@ def __init__(self, scheduler, follow_interval=20000, **kwargs):
x_axis_type="datetime",
tools=tools,
x_range=x_range,
id="bk-worker-disk-ts",
# id="bk-worker-disk-ts",
name="worker_disk-timeseries",
**kwargs,
)
Expand Down Expand Up @@ -1420,7 +1420,7 @@ def __init__(self, scheduler, **kwargs):
fig = figure(
title="Compute Time Per Task",
tools="",
id="bk-Compute-by-key-plot",
# id="bk-Compute-by-key-plot",
name="compute_time_per_key",
x_range=["a", "b"],
**kwargs,
Expand Down Expand Up @@ -1465,12 +1465,12 @@ def __init__(self, scheduler, **kwargs):
)

self.fig = fig
tab1 = Panel(child=fig, title="Bar Chart")
tab1 = TabPanel(child=fig, title="Bar Chart")

fig2 = figure(
title="Compute Time Per Task",
tools="",
id="bk-Compute-by-key-pie",
# id="bk-Compute-by-key-pie",
name="compute_time_per_key-pie",
x_range=(-0.5, 1.0),
**kwargs,
Expand Down Expand Up @@ -1509,7 +1509,7 @@ def __init__(self, scheduler, **kwargs):
hover.point_policy = "follow_mouse"
fig2.add_tools(hover)
self.wedge_fig = fig2
tab2 = Panel(child=fig2, title="Pie Chart")
tab2 = TabPanel(child=fig2, title="Pie Chart")

self.root = Tabs(tabs=[tab1, tab2])

Expand Down Expand Up @@ -1579,7 +1579,7 @@ def __init__(self, scheduler, **kwargs):
self.root = figure(
title="Aggregate Per Action",
tools="",
id="bk-aggregate-per-action-plot",
# id="bk-aggregate-per-action-plot",
name="aggregate_per_action",
x_range=["a", "b"],
**kwargs,
Expand Down Expand Up @@ -1671,7 +1671,7 @@ def __init__(self, scheduler, **kwargs):
self.root = figure(
title="Memory Use",
tools="",
id="bk-memory-by-key-plot",
# id="bk-memory-by-key-plot",
name="memory_by_key",
x_range=["a", "b"],
**kwargs,
Expand Down Expand Up @@ -1748,7 +1748,7 @@ def __init__(self, scheduler, width=600, **kwargs):
processing = figure(
title="Tasks Processing",
tools="",
id="bk-nprocessing-plot",
# id="bk-nprocessing-plot",
name="processing",
width=int(width / 2),
min_border_bottom=50,
Expand All @@ -1768,7 +1768,7 @@ def __init__(self, scheduler, width=600, **kwargs):
cpu = figure(
title="CPU Utilization",
tools="",
id="bk-cpu-worker-plot",
# id="bk-cpu-worker-plot",
width=int(width / 2),
name="cpu_hist",
x_range=(0, 100),
Expand Down Expand Up @@ -2187,7 +2187,7 @@ def task_stream_figure(clear_interval="20s", **kwargs):
root = figure(
name="task_stream",
title="Task Stream",
id="bk-task-stream-plot",
# id="bk-task-stream-plot",
x_range=x_range,
y_range=y_range,
toolbar_location="above",
Expand Down Expand Up @@ -2902,7 +2902,7 @@ def __init__(self, scheduler, **kwargs):
y_range = Range1d(0, max(self.plugin.nthreads))

self.root = figure(
id="bk-task-group-progress-plot",
# id="bk-task-group-progress-plot",
Copy link
Contributor

@bryevdv bryevdv Oct 24, 2022 •
edited

Choose a reason for hiding this comment

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

A better alternative that should also work with 2.x is to set name instead of id. AFAIK that ill add a name class to the element that can be used for CSS selection. (cc @mattpap).

Edit: or of these are not being used for anything in particular, then they can just be omitted entirely

Copy link
Member Author

Choose a reason for hiding this comment

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

Good to know. I tried that initially, but it looks like we're defining a name and id for some figures already (not sure why). I wanted to see if CI breaks if we just don't specify an id at all

Copy link
Contributor

@bryevdv bryevdv Oct 24, 2022 •
edited

Choose a reason for hiding this comment

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

alternatively css_classes is still also an option

https://docs.bokeh.org/en/dev-3.0/docs/reference/models/layouts.html#bokeh.models.LayoutDOM.css_classes

Copy link
Member Author

Choose a reason for hiding this comment

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

@ian-r-rose do you have any insight here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I took a brief look through the dashboard codebase and didn't see anywhere where the DOM ids are explicitly used, so I think it should be relatively safe to just remove these id= parameters.

I do see a couple of places where bokeh components have custom CSS styling, and that will likely need some updating here. But that is a separate concern from the id parameters here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do see a couple of places where bokeh components have custom CSS styling, and that will likely need some updating here

Could you point me to those?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm referring to these:

distributed/distributed/http/static/css/base.css

Lines 100 to 107 in 5dccad4

.bk-root .bk-toolbar-box .bk-toolbar-right {
top: 4px;
right: 4px;
}
.bk-root .bk-data-table {
z-index: 0;
}

I'm not sure if they will work with Bokeh 3, but my read of the changelog is that the CSS system has been overhauled, so it may be worth verifying.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mattpap can you advise?

Copy link
Contributor

Choose a reason for hiding this comment

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

Due to migration to shadow DOM (or web components, or modularized CSS; different names same idea), styling of bokeh's components with external style sheets is not possible anymore. Each component has to be styled individually. Many CSS classes are gone (e.g. .bk-toolbar-box) or renamed (simplified) (e.g. .bk-toolbar-right becomes .bk-right), because complex selectors are not needed anymore with modularized CSS. .bk-root is gone, because it's not needed anymore. Also the general structure of bokehjs components' DOM and CSS changed significantly, so even if custom CSS is updated to work with shadow DOM, it still will likely not do what's expected without additional changes.

If you have only individual (instances) components to style, then this should work (give or take, I didn't test it):

.bk-root .bk-toolbar-box .bk-toolbar-right {
  top: 4px;
  right: 4px;
}
Toolbar(stylesheets=["""
:host(.bk-right) {
  top: 4px;
  right: 4px;
}
"""])
.bk-root .bk-data-table {
  z-index: 0;
}
DataTable(stylesheets=["""
.bk-data-table {
  z-index: 0;
}
"""])

Most likely you don't need the second one, as we hopefully fixed all issues with overlapping components.

If you want to style all components of a given kind, like all toolbars, then we don't have a good replacement right now. It's possible to configure stylesheets property via a theme, but we are still working one something more robust. The current advice is that, if you have a lot of custom CSS, then probably you will have to postpone migration to 3.0 for a moment (until 3.0.n or 3.1; in particular in 3.1 we intend to define an "API" for bokehjs' CSS). If you have just a little bit of customization, then we can figure it out on case-by-case basis.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the context and code snippets @mattpap!

Many CSS classes are gone (e.g. .bk-toolbar-box) or renamed (simplified) (e.g. .bk-toolbar-right becomes .bk-right)

Grepping through distributed I don't see us using Toolbar object. Maybe this CSS isn't needed anymore?

Most likely you don't need the second one, as we hopefully fixed all issues with overlapping components.

I found that without adding the DataTable CSS I ran into #5136 (the original issue why this custom CSS was added). Thanks again for adding the exact I needed to keep this CSS around for bokeh=3 : )

title="Task Group Progress",
name="task_group_progress",
toolbar_location="above",
Expand Down Expand Up @@ -3175,7 +3175,7 @@ def __init__(self, scheduler, **kwargs):
y_range = Range1d(-8, 0)

self.root = figure(
id="bk-task-progress-plot",
# id="bk-task-progress-plot",
title="Progress",
name="task_progress",
x_range=x_range,
Expand Down Expand Up @@ -3709,7 +3709,8 @@ def __init__(self, scheduler, width=800, **kwargs):
if self.extra_names:
components.append(extra_table)

self.root = column(*components, id="bk-worker-table", **sizing_mode)
self.root = column(*components, **sizing_mode)
# self.root = column(*components, id="bk-worker-table", **sizing_mode)

@without_property_validation
def update(self):
Expand Down Expand Up @@ -4061,16 +4062,16 @@ def __init__(self, scheduler, start=None):

self.root = Div(
text=logs_html,
style={
"width": "100%",
"height": "100%",
"max-width": "1920px",
"max-height": "1080px",
"padding": "12px",
"border": "1px solid lightgray",
"box-shadow": "inset 1px 0 8px 0 lightgray",
"overflow": "auto",
},
# **{
# "width": "100%",
# "height": "100%",
# "max-width": "1920px",
# "max-height": "1080px",
# "padding": "12px",
# "border": "1px solid lightgray",
# "box-shadow": "inset 1px 0 8px 0 lightgray",
# "overflow": "auto",
# },
Copy link
Contributor

@bryevdv bryevdv Oct 24, 2022 •
edited

Choose a reason for hiding this comment

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

I think you can now accomplish this similar to:

https://github.com/bokeh/bokeh/blob/branch-3.0/examples/basic/layouts/sizing_mode.py#L19

container.stylesheets.append(":host { border: 10px solid grey; }")

cc @mattpap for the best approach to support both 2.x and 3.x

Copy link
Contributor

Choose a reason for hiding this comment

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

UIElement.styles is the replacement (works for all UI elements not just markup widgets). The name is styles and not style, so that people don't confuse this with DOM style attribute. I suppose we could have a deprecated alias in markup widgets for some time in 3.x.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @mattpap! I think 8d8f5f3 should address this one, though let me know if you think I've misunderstood your comment

)


Expand Down Expand Up @@ -4281,10 +4282,10 @@ def status_doc(scheduler, extra, doc):

doc.add_root(workers_memory.root)

tab1 = Panel(child=processing_root, title="Processing")
tab2 = Panel(child=cpu_root, title="CPU")
tab3 = Panel(child=occupancy_root, title="Occupancy")
tab4 = Panel(child=workers_transfer_bytes.root, title="Data Transfer")
tab1 = TabPanel(child=processing_root, title="Processing")
tab2 = TabPanel(child=cpu_root, title="CPU")
tab3 = TabPanel(child=occupancy_root, title="Occupancy")
tab4 = TabPanel(child=workers_transfer_bytes.root, title="Data Transfer")

proc_tabs = Tabs(tabs=[tab1, tab2, tab3, tab4], name="processing_tabs")
doc.add_root(proc_tabs)
Expand Down
11 changes: 9 additions & 2 deletions distributed/dashboard/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import functools
import warnings

import bokeh
from bokeh.application import Application
from bokeh.application.handlers.function import FunctionHandler
from bokeh.server.server import BokehTornado
Expand All @@ -12,14 +11,22 @@

import dask

if parse_version(bokeh.__version__) < parse_version("2.1.1"):
from distributed.dashboard.utils import BOKEH_VERSION

if BOKEH_VERSION < parse_version("2.1.1"):
warnings.warn(
"\nDask needs bokeh >= 2.1.1 for the dashboard."
"\nContinuing without the dashboard."
)
raise ImportError("Dask needs bokeh >= 2.1.1")


if BOKEH_VERSION.major < 3:
from bokeh.models import Panel as TabPanel # noqa: F401
else:
from bokeh.models import TabPanel # noqa: F401


def BokehApplication(applications, server, prefix="/", template_variables=None):
template_variables = template_variables or {}
prefix = "/" + prefix.strip("/") + "/" if prefix else "/"
Expand Down
4 changes: 2 additions & 2 deletions distributed/dashboard/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
import bokeh
from bokeh.core.properties import without_property_validation
from bokeh.io import curdoc
from packaging.version import parse as parse_version
from packaging.version import Version
from tlz.curried import first

try:
import numpy as np
except ImportError:
np = None # type: ignore

BOKEH_VERSION = parse_version(bokeh.__version__)
BOKEH_VERSION = Version(bokeh.__version__)

PROFILING = False

Expand Down