From 33f8b2a2289eec6c5ef1087b442b36b1f977dea2 Mon Sep 17 00:00:00 2001 From: SangBin Cho Date: Thu, 18 May 2023 03:12:13 +0900 Subject: [PATCH] Revert "Add a disconnect button to the context widgets in notebooks (#34815)" (#35426) This reverts commit f31d70e6be5db7432e1f82c87bde2c3b410f9552. --- python/ray/_private/worker.py | 90 ++++--------------- python/ray/client_builder.py | 15 ++++ python/ray/tests/test_widgets.py | 4 - python/ray/widgets/render.py | 5 +- python/ray/widgets/templates/context.html.j2 | 37 +++++++- .../widgets/templates/context_logo.html.j2 | 13 --- .../widgets/templates/context_table.html.j2 | 11 --- python/ray/widgets/util.py | 16 ++-- 8 files changed, 77 insertions(+), 114 deletions(-) delete mode 100644 python/ray/widgets/templates/context_logo.html.j2 delete mode 100644 python/ray/widgets/templates/context_table.html.j2 diff --git a/python/ray/_private/worker.py b/python/ray/_private/worker.py index 79b1af201f69..69a8327173c9 100644 --- a/python/ray/_private/worker.py +++ b/python/ray/_private/worker.py @@ -96,7 +96,6 @@ from ray.util.scheduling_strategies import PlacementGroupSchedulingStrategy from ray.util.tracing.tracing_helper import _import_from_string from ray.widgets import Template -from ray.widgets.util import ensure_ipywidgets_dep SCRIPT_MODE = 0 WORKER_MODE = 1 @@ -1020,10 +1019,6 @@ class BaseContext(metaclass=ABCMeta): Base class for RayContext and ClientContext """ - dashboard_url: Optional[str] - python_version: str - ray_version: str - @abstractmethod def disconnect(self): """ @@ -1041,73 +1036,6 @@ def __enter__(self): def __exit__(self): pass - def _context_table_template(self): - if self.dashboard_url: - dashboard_row = Template("context_dashrow.html.j2").render( - dashboard_url="http://" + self.dashboard_url - ) - else: - dashboard_row = None - - return Template("context_table.html.j2").render( - python_version=self.python_version, - ray_version=self.ray_version, - dashboard_row=dashboard_row, - ) - - def _repr_html_(self): - return Template("context.html.j2").render( - context_logo=Template("context_logo.html.j2").render(), - context_table=self._context_table_template(), - ) - - @ensure_ipywidgets_dep("8") - def _get_widget_bundle(self, **kwargs) -> Dict[str, Any]: - """Get the mimebundle for the widget representation of the context. - - Args: - **kwargs: Passed to the _repr_mimebundle_() function for the widget - - Returns: - Dictionary ("mimebundle") of the widget representation of the context. - """ - import ipywidgets - - disconnect_button = ipywidgets.Button( - description="Disconnect", - disabled=False, - button_style="", - tooltip="Disconnect from the Ray cluster", - layout=ipywidgets.Layout(margin="auto 0px 0px 0px"), - ) - - def disconnect_callback(button): - button.disabled = True - button.description = "Disconnecting..." - self.disconnect() - button.description = "Disconnected" - - disconnect_button.on_click(disconnect_callback) - left_content = ipywidgets.VBox( - [ - ipywidgets.HTML(Template("context_logo.html.j2").render()), - disconnect_button, - ], - layout=ipywidgets.Layout(), - ) - right_content = ipywidgets.HTML(self._context_table_template()) - widget = ipywidgets.HBox( - [left_content, right_content], layout=ipywidgets.Layout(width="100%") - ) - return widget._repr_mimebundle_(**kwargs) - - def _repr_mimebundle_(self, **kwargs): - bundle = self._get_widget_bundle(**kwargs) - - # Overwrite the widget html repr and default repr with those of the BaseContext - bundle.update({"text/html": self._repr_html_(), "text/plain": repr(self)}) - return bundle - @dataclass class RayContext(BaseContext, Mapping): @@ -1119,10 +1047,10 @@ class RayContext(BaseContext, Mapping): python_version: str ray_version: str ray_commit: str - protocol_version: Optional[str] + protocol_version = Optional[str] + address_info: Dict[str, Optional[str]] def __init__(self, address_info: Dict[str, Optional[str]]): - super().__init__() self.dashboard_url = get_dashboard_url() self.python_version = "{}.{}.{}".format(*sys.version_info[:3]) self.ray_version = ray.__version__ @@ -1164,6 +1092,20 @@ def disconnect(self): # Include disconnect() to stay consistent with ClientContext ray.shutdown() + def _repr_html_(self): + if self.dashboard_url: + dashboard_row = Template("context_dashrow.html.j2").render( + dashboard_url="http://" + self.dashboard_url + ) + else: + dashboard_row = None + + return Template("context.html.j2").render( + python_version=self.python_version, + ray_version=self.ray_version, + dashboard_row=dashboard_row, + ) + global_worker = Worker() """Worker: The global Worker object for this worker process. diff --git a/python/ray/client_builder.py b/python/ray/client_builder.py index d5cbc03142e1..4e379dc5c5b6 100644 --- a/python/ray/client_builder.py +++ b/python/ray/client_builder.py @@ -19,6 +19,7 @@ from ray._private.worker import init as ray_driver_init from ray.job_config import JobConfig from ray.util.annotations import Deprecated, PublicAPI +from ray.widgets import Template logger = logging.getLogger(__name__) @@ -85,6 +86,20 @@ def _disconnect_with_context(self, force_disconnect: bool) -> None: # This is only a driver connected to an existing cluster. ray.shutdown() + def _repr_html_(self): + if self.dashboard_url: + dashboard_row = Template("context_dashrow.html.j2").render( + dashboard_url="http://" + self.dashboard_url + ) + else: + dashboard_row = None + + return Template("context.html.j2").render( + python_version=self.python_version, + ray_version=self.ray_version, + dashboard_row=dashboard_row, + ) + @Deprecated class ClientBuilder: diff --git a/python/ray/tests/test_widgets.py b/python/ray/tests/test_widgets.py index 3697516def3a..ad95d2b3c9e4 100644 --- a/python/ray/tests/test_widgets.py +++ b/python/ray/tests/test_widgets.py @@ -1,4 +1,3 @@ -import logging from unittest import mock import pytest @@ -8,7 +7,6 @@ @mock.patch("importlib.import_module") def test_ensure_notebook_dep_missing(mock_import_module, caplog): """Test that missing notebook dependencies trigger a warning.""" - caplog.set_level(logging.INFO) class MockDep: __version__ = "8.0.0" @@ -32,7 +30,6 @@ def dummy_ipython_display(self): @mock.patch("importlib.import_module") def test_ensure_notebook_dep_outdated(mock_import_module, caplog): """Test that outdated notebook dependencies trigger a warning.""" - caplog.set_level(logging.INFO) class MockDep: __version__ = "7.0.0" @@ -52,7 +49,6 @@ def dummy_ipython_display(): @mock.patch("importlib.import_module") def test_ensure_notebook_valid(mock_import_module, caplog): """Test that valid notebook dependencies don't trigger a warning.""" - caplog.set_level(logging.INFO) class MockDep: __version__ = "8.0.0" diff --git a/python/ray/widgets/render.py b/python/ray/widgets/render.py index f9e861d39925..1fc0c69d5f0f 100644 --- a/python/ray/widgets/render.py +++ b/python/ray/widgets/render.py @@ -19,7 +19,7 @@ def render(self, **kwargs) -> str: from the keyword arguments. Returns: - HTML template with the keys of the kwargs replaced with corresponding + str: HTML template with the keys of the kwargs replaced with corresponding values. """ rendered = self.template @@ -34,6 +34,7 @@ def list_templates() -> List[pathlib.Path]: """List the available HTML templates. Returns: - A list of files with .html.j2 extensions inside ../templates/ + List[pathlib.Path]: A list of files with .html.j2 extensions inside + ./templates/ """ return (pathlib.Path(__file__).parent / "templates").glob("*.html.j2") diff --git a/python/ray/widgets/templates/context.html.j2 b/python/ray/widgets/templates/context.html.j2 index 26cc0ef6c878..3e664e01eae2 100644 --- a/python/ray/widgets/templates/context.html.j2 +++ b/python/ray/widgets/templates/context.html.j2 @@ -1,6 +1,37 @@ -
+
- {{ context_logo }} - {{ context_table }} +

Ray

+ + + + + + + + + + + + + + + {{ dashboard_row }} +
Python version:{{ python_version }}
Ray version: {{ ray_version }}
diff --git a/python/ray/widgets/templates/context_logo.html.j2 b/python/ray/widgets/templates/context_logo.html.j2 deleted file mode 100644 index 9233fe3a7722..000000000000 --- a/python/ray/widgets/templates/context_logo.html.j2 +++ /dev/null @@ -1,13 +0,0 @@ - diff --git a/python/ray/widgets/templates/context_table.html.j2 b/python/ray/widgets/templates/context_table.html.j2 deleted file mode 100644 index d06822d0c1f5..000000000000 --- a/python/ray/widgets/templates/context_table.html.j2 +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - - - - {{ dashboard_row }} - diff --git a/python/ray/widgets/util.py b/python/ray/widgets/util.py index 3fbe3b4aa9ce..6991384779f2 100644 --- a/python/ray/widgets/util.py +++ b/python/ray/widgets/util.py @@ -73,9 +73,9 @@ def ensure_notebook_deps( ) -> Callable[[F], F]: """Generate a decorator which checks for soft dependencies. - This decorator is meant to wrap _repr_mimebundle_ methods. If the dependency is not - found, or a version is specified here and the version of the package is older than - the specified version, the original repr is used. + This decorator is meant to wrap repr methods. If the dependency is not found, + or a version is specified here and the version of the package is older than the + specified version, the original repr is used. If the dependency is missing or the version is old, a log message is displayed. Args: @@ -155,11 +155,11 @@ def _has_missing( message = f"Run `pip install {' '.join(missing)}` for rich notebook output." if sys.version_info < (3, 8): - logger.info(f"Missing packages: {missing}. {message}") + logger.warning(f"Missing packages: {missing}. {message}") else: # stacklevel=3: First level is this function, then ensure_notebook_deps, # then the actual function affected. - logger.info(f"Missing packages: {missing}. {message}", stacklevel=3) + logger.warning(f"Missing packages: {missing}. {message}", stacklevel=3) return missing @@ -190,11 +190,13 @@ def _has_outdated( message = f"Run `pip install -U {install_str}` for rich notebook output." if sys.version_info < (3, 8): - logger.info(f"Outdated packages:\n{outdated_str}\n{message}") + logger.warning(f"Outdated packages:\n{outdated_str}\n{message}") else: # stacklevel=3: First level is this function, then ensure_notebook_deps, # then the actual function affected. - logger.info(f"Outdated packages:\n{outdated_str}\n{message}", stacklevel=3) + logger.warning( + f"Outdated packages:\n{outdated_str}\n{message}", stacklevel=3 + ) return outdated