From 75614e7885110cb0b71e5b5ac8632abc0b7938da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 2 Jun 2023 21:32:44 +0200 Subject: [PATCH] Show notifications for new app version and import warnings (#1955) --- lib/livebook/app.ex | 32 ++++++++++++++----- lib/livebook/apps.ex | 24 +++++++++----- lib/livebook/live_markdown/import.ex | 6 ++-- lib/livebook/session.ex | 8 ++++- .../components/core_components.ex | 28 ++++++++++++++++ lib/livebook_web/live/app_session_live.ex | 8 +++++ lib/livebook_web/live/apps_live.ex | 3 ++ .../live/session_live/app_info_component.ex | 11 ++++--- test/livebook/apps_test.exs | 26 +++++++++++++++ test/livebook/live_markdown/import_test.exs | 6 ++-- test/livebook_web/live/open_live_test.exs | 2 +- 11 files changed, 125 insertions(+), 29 deletions(-) diff --git a/lib/livebook/app.ex b/lib/livebook/app.ex index ca95e82d8..78790ed94 100644 --- a/lib/livebook/app.ex +++ b/lib/livebook/app.ex @@ -13,12 +13,22 @@ defmodule Livebook.App do # always taken from the most recently deployed notebook (e.g. access # type, automatic shutdown, deployment strategy). - defstruct [:slug, :pid, :version, :notebook_name, :public?, :multi_session, :sessions] + defstruct [ + :slug, + :pid, + :version, + :warnings, + :notebook_name, + :public?, + :multi_session, + :sessions + ] @type t :: %{ slug: slug(), pid: pid(), version: pos_integer(), + warnings: list(String.t()), notebook_name: String.t(), public?: boolean(), multi_session: boolean(), @@ -46,12 +56,15 @@ defmodule Livebook.App do * `:notebook` (required) - the notebook for initial deployment + * `:warnings` - a list of warnings to show for the initial deployment + """ @spec start_link(keyword()) :: {:ok, pid} | {:error, any()} def start_link(opts) do notebook = Keyword.fetch!(opts, :notebook) + warnings = Keyword.get(opts, :warnings, []) - GenServer.start_link(__MODULE__, {notebook}) + GenServer.start_link(__MODULE__, {notebook, warnings}) end @doc """ @@ -100,9 +113,10 @@ defmodule Livebook.App do @doc """ Deploys a new notebook into the app. """ - @spec deploy(pid(), Livebook.Notebook.t()) :: :ok - def deploy(pid, notebook) do - GenServer.cast(pid, {:deploy, notebook}) + @spec deploy(pid(), Livebook.Notebook.t(), keyword()) :: :ok + def deploy(pid, notebook, opts \\ []) do + warnings = Keyword.get(opts, :warnings, []) + GenServer.cast(pid, {:deploy, notebook, warnings}) end @doc """ @@ -136,11 +150,12 @@ defmodule Livebook.App do end @impl true - def init({notebook}) do + def init({notebook, warnings}) do {:ok, %{ version: 1, notebook: notebook, + warnings: warnings, sessions: [], users: %{} } @@ -176,11 +191,11 @@ defmodule Livebook.App do end @impl true - def handle_cast({:deploy, notebook}, state) do + def handle_cast({:deploy, notebook, warnings}, state) do true = notebook.app_settings.slug == state.notebook.app_settings.slug {:noreply, - %{state | notebook: notebook, version: state.version + 1} + %{state | notebook: notebook, version: state.version + 1, warnings: warnings} |> start_eagerly() |> shutdown_old_versions() |> notify_update()} @@ -221,6 +236,7 @@ defmodule Livebook.App do slug: state.notebook.app_settings.slug, pid: self(), version: state.version, + warnings: state.warnings, notebook_name: state.notebook.name, public?: state.notebook.app_settings.access_type == :public, multi_session: state.notebook.app_settings.multi_session, diff --git a/lib/livebook/apps.ex b/lib/livebook/apps.ex index 60ce553bd..b5e3eebe3 100644 --- a/lib/livebook/apps.ex +++ b/lib/livebook/apps.ex @@ -16,9 +16,16 @@ defmodule Livebook.Apps do If there is no app process under the corresponding slug, it is started. Otherwise the notebook is deployed as a new version into the existing app. + + ## Options + + * `:warnings` - a list of warnings to show for the new deployment + """ - @spec deploy(Livebook.Notebook.t()) :: {:ok, pid()} | {:error, term()} - def deploy(notebook) do + @spec deploy(Livebook.Notebook.t(), keyword()) :: {:ok, pid()} | {:error, term()} + def deploy(notebook, opts \\ []) do + opts = Keyword.validate!(opts, warnings: []) + slug = notebook.app_settings.slug name = name(slug) @@ -27,25 +34,25 @@ defmodule Livebook.Apps do :global.trans({{:app_registration, name}, node()}, fn -> case :global.whereis_name(name) do :undefined -> - with {:ok, pid} <- start_app(notebook) do + with {:ok, pid} <- start_app(notebook, opts[:warnings]) do :yes = :global.register_name(name, pid) {:ok, pid} end pid -> - App.deploy(pid, notebook) + App.deploy(pid, notebook, warnings: opts[:warnings]) {:ok, pid} end end) pid -> - App.deploy(pid, notebook) + App.deploy(pid, notebook, warnings: opts[:warnings]) {:ok, pid} end end - defp start_app(notebook) do - opts = [notebook: notebook] + defp start_app(notebook, warnings) do + opts = [notebook: notebook, warnings: warnings] case DynamicSupervisor.start_child(Livebook.AppSupervisor, {App, opts}) do {:ok, pid} -> @@ -178,7 +185,8 @@ defmodule Livebook.Apps do end if Livebook.Notebook.AppSettings.valid?(notebook.app_settings) do - deploy(notebook) + warnings = Enum.map(warnings, &("Import: " <> &1)) + deploy(notebook, warnings: warnings) else Logger.warning("Skipping app deployment at #{path} due to invalid settings") end diff --git a/lib/livebook/live_markdown/import.ex b/lib/livebook/live_markdown/import.ex index 9c99b8899..bda72b4cf 100644 --- a/lib/livebook/live_markdown/import.ex +++ b/lib/livebook/live_markdown/import.ex @@ -21,7 +21,7 @@ defmodule Livebook.LiveMarkdown.Import do end defp earmark_message_to_string({_severity, line_number, message}) do - "Line #{line_number}: #{message}" + "line #{line_number} - #{Livebook.Utils.downcase_first(message)}" end # Does initial pre-processing of the AST, so that it conforms to the expected form. @@ -46,7 +46,7 @@ defmodule Livebook.LiveMarkdown.Import do ast = Enum.map(ast, &downgrade_heading/1) message = - "Downgrading all headings, because #{primary_headings} instances of heading 1 were found" + "downgrading all headings, because #{primary_headings} instances of heading 1 were found" {ast, [message]} else @@ -75,7 +75,7 @@ defmodule Livebook.LiveMarkdown.Import do {ast, []} else ast = comments ++ [heading] ++ leading ++ rest - message = "Moving heading 1 to the top of the notebook" + message = "moving heading 1 to the top of the notebook" {ast, [message]} end end diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 68f4254da..ecad8a494 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -2040,9 +2040,15 @@ defmodule Livebook.Session do locator = {container_ref_for_section(section), cell.id} parent_locators = parent_locators_for_cell(state.data, cell) + language = + case cell do + %Cell.Code{} -> cell.language + _ -> :elixir + end + Runtime.evaluate_code( state.data.runtime, - cell.language, + language, cell.source, locator, parent_locators, diff --git a/lib/livebook_web/components/core_components.ex b/lib/livebook_web/components/core_components.ex index 7b968ea5b..c83fe2a73 100644 --- a/lib/livebook_web/components/core_components.ex +++ b/lib/livebook_web/components/core_components.ex @@ -85,6 +85,34 @@ defmodule LivebookWeb.CoreComponents do """ end + @doc """ + Renders a message notice. + + Similar to `flash/1`, but for permanent messages on the page. + + ## Examples + + <.message_box kind={:info} message="🦊 in a 📦" /> + + """ + + attr :message, :string, required: true + attr :kind, :atom, values: [:info, :success, :warning, :error] + + def message_box(assigns) do + ~H""" +
+
<%= @message %>
+
+ """ + end + @doc """ Creates a live region with the given role. diff --git a/lib/livebook_web/live/app_session_live.ex b/lib/livebook_web/live/app_session_live.ex index 4bdd6b1a3..656d0d504 100644 --- a/lib/livebook_web/live/app_session_live.ex +++ b/lib/livebook_web/live/app_session_live.ex @@ -251,6 +251,14 @@ defmodule LivebookWeb.AppSessionLive do redirect_on_closed(socket) end + defp after_operation(socket, _prev_socket, {:app_shutdown, _client_id}) do + put_flash( + socket, + :info, + "A new version has been deployed, this session will close once everybody leaves" + ) + end + defp after_operation(socket, _prev_socket, _operation), do: socket defp redirect_on_closed(socket) do diff --git a/lib/livebook_web/live/apps_live.ex b/lib/livebook_web/live/apps_live.ex index 3fac68c80..735a90208 100644 --- a/lib/livebook_web/live/apps_live.ex +++ b/lib/livebook_web/live/apps_live.ex @@ -53,6 +53,9 @@ defmodule LivebookWeb.AppsLive do
<%= "/" <> app.slug %>
+
+ <.message_box :for={warning <- app.warnings} kind={:warning} message={warning} /> +
App info
diff --git a/lib/livebook_web/live/session_live/app_info_component.ex b/lib/livebook_web/live/session_live/app_info_component.ex index bb032a18b..7fc0e7b46 100644 --- a/lib/livebook_web/live/session_live/app_info_component.ex +++ b/lib/livebook_web/live/session_live/app_info_component.ex @@ -14,11 +14,12 @@ defmodule LivebookWeb.SessionLive.AppInfoComponent do <.app_info_icon /> <%= if @session.mode == :app do %> -
- - This session is a running app. To deploy a modified version, you can fork it. - -
+
+ <.message_box + kind={:info} + message="This session is a running app. To deploy a modified version, you can fork it." + /> +