From 75e3447ce2ff8c9f6055a096b7a49848eb3f6923 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Wed, 9 Aug 2023 16:00:56 -0300 Subject: [PATCH 01/35] New User should use the same id from session data --- lib/livebook/users/user.ex | 6 +++--- lib/livebook_web/live/hooks/user_hook.ex | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/livebook/users/user.ex b/lib/livebook/users/user.ex index 4c071631843..a58b1303088 100644 --- a/lib/livebook/users/user.ex +++ b/lib/livebook/users/user.ex @@ -33,10 +33,10 @@ defmodule Livebook.Users.User do @doc """ Generates a new user. """ - @spec new() :: t() - def new() do + @spec new(String.t()) :: t() + def new(id \\ Utils.random_id()) do %__MODULE__{ - id: Utils.random_id(), + id: id, name: nil, email: nil, hex_color: Livebook.EctoTypes.HexColor.random() diff --git a/lib/livebook_web/live/hooks/user_hook.ex b/lib/livebook_web/live/hooks/user_hook.ex index 5c37fe3a1a3..7d90458497a 100644 --- a/lib/livebook_web/live/hooks/user_hook.ex +++ b/lib/livebook_web/live/hooks/user_hook.ex @@ -33,9 +33,7 @@ defmodule LivebookWeb.UserHook do # attributes if the socket is connected. Otherwise uses # `user_data` from session. defp build_current_user(session, socket) do - user = User.new() identity_data = Map.new(session["identity_data"], fn {k, v} -> {Atom.to_string(k), v} end) - connect_params = get_connect_params(socket) || %{} attrs = connect_params["user_data"] || session["user_data"] || %{} @@ -45,6 +43,8 @@ defmodule LivebookWeb.UserHook do attrs -> attrs end + user = User.new(attrs["id"]) + case Livebook.Users.update_user(user, attrs) do {:ok, user} -> user {:error, _changeset} -> user From 7fc0e647f31cd4b488a5fb9411b492c5ad10db93 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Wed, 13 Sep 2023 11:57:47 -0300 Subject: [PATCH 02/35] Make `create_teams_file_system/2` public --- test/livebook_teams/teams_test.exs | 6 ------ test/support/hub_helpers.ex | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/livebook_teams/teams_test.exs b/test/livebook_teams/teams_test.exs index 99008877923..7963239dcd2 100644 --- a/test/livebook_teams/teams_test.exs +++ b/test/livebook_teams/teams_test.exs @@ -303,10 +303,4 @@ defmodule Livebook.TeamsTest do "Something went wrong, try again later or please file a bug if it persists"} end end - - defp create_teams_file_system(hub, node) do - org_key = :erpc.call(node, Hub.Integration, :get_org_key!, [hub.org_key_id]) - - :erpc.call(node, Hub.Integration, :create_file_system, [[org_key: org_key]]) - end end diff --git a/test/support/hub_helpers.ex b/test/support/hub_helpers.ex index a88cc3baa22..303dac0c0b7 100644 --- a/test/support/hub_helpers.ex +++ b/test/support/hub_helpers.ex @@ -102,6 +102,12 @@ defmodule Livebook.HubHelpers do send(pid, {:event, :secret_deleted, secret_deleted}) end + def create_teams_file_system(hub, node) do + org_key = :erpc.call(node, Hub.Integration, :get_org_key!, [hub.org_key_id]) + + :erpc.call(node, Hub.Integration, :create_file_system, [[org_key: org_key]]) + end + defp hub_pid(hub) do if pid = GenServer.whereis({:via, Registry, {Livebook.HubsRegistry, hub.id}}) do {:ok, pid} From b1d8ca9690e7f6036c13125f9e7f7e09d0fca249 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Wed, 13 Sep 2023 11:59:57 -0300 Subject: [PATCH 03/35] Add file system topic docs --- lib/livebook/hubs.ex | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/livebook/hubs.ex b/lib/livebook/hubs.ex index f5ac3a112f6..bf06902056e 100644 --- a/lib/livebook/hubs.ex +++ b/lib/livebook/hubs.ex @@ -159,6 +159,12 @@ defmodule Livebook.Hubs do * `{:secret_updated, %Secret{}}` * `{:secret_deleted, %Secret{}}` + Topic `hubs:file_systems`: + + * `{:file_system_created, t:FileSystem}` + * `{:file_system_updated, t:FileSystem}` + * `{:file_system_deleted, t:FileSystem}` + """ @spec subscribe(atom() | list(atom())) :: :ok | {:error, term()} def subscribe(topics) when is_list(topics) do From 1a0187085683f2b5dc3a7bb202a0f01c02508b70 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Tue, 8 Aug 2023 16:49:57 -0300 Subject: [PATCH 04/35] Add function to list file systems from all hubs --- lib/livebook/hubs.ex | 11 +++++++++++ lib/livebook_web/live/file_select_component.ex | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/livebook/hubs.ex b/lib/livebook/hubs.ex index bf06902056e..1bf2d7fff2a 100644 --- a/lib/livebook/hubs.ex +++ b/lib/livebook/hubs.ex @@ -302,6 +302,17 @@ defmodule Livebook.Hubs do Provider.verify_notebook_stamp(hub, notebook_source, stamp) end + @doc """ + Gets a list of file systems from all hubs. + """ + @spec get_file_systems() :: list(FileSystem.t()) + def get_file_systems() do + file_systems = Enum.flat_map(get_hubs(), &Provider.get_file_systems/1) + local_file_system = Livebook.Config.local_file_system() + + [local_file_system | Enum.sort_by(file_systems, & &1.id)] + end + @doc """ Gets a list of file systems for given hub. """ diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index 8a844ccadf6..eae07e3e2c8 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -45,7 +45,7 @@ defmodule LivebookWeb.FileSelectComponent do renaming_file: nil, renamed_name: nil, error_message: nil, - file_systems: Livebook.Settings.file_systems() + file_systems: [] ) |> allow_upload(:folder, accept: :any, From df1a0c22af2c6c06fdb84b2b16e4b0ce0ec2715e Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Wed, 9 Aug 2023 17:35:42 -0300 Subject: [PATCH 05/35] Use the notebook hub to list file systems --- lib/livebook_web/live/session_live.ex | 11 ++++++++++- .../session_live/add_file_entry_file_component.ex | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 328f90368e9..224363ddde3 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -488,7 +488,12 @@ defmodule LivebookWeb.SessionLive do width={:big} patch={@self_path} > - <.add_file_entry_content session={@session} file_entries={@data_view.file_entries} tab={@tab} /> + <.add_file_entry_content + session={@session} + hub={@data_view.hub} + file_entries={@data_view.file_entries} + tab={@tab} + /> <.modal @@ -947,24 +952,28 @@ defmodule LivebookWeb.SessionLive do :if={@tab == "storage"} module={LivebookWeb.SessionLive.AddFileEntryFileComponent} id="add-file-entry-from-file" + hub={@hub} session={@session} /> <.live_component :if={@tab == "url"} module={LivebookWeb.SessionLive.AddFileEntryUrlComponent} id="add-file-entry-from-url" + hub={@hub} session={@session} /> <.live_component :if={@tab == "upload"} module={LivebookWeb.SessionLive.AddFileEntryUploadComponent} id="add-file-entry-from-upload" + hub={@hub} session={@session} /> <.live_component :if={@tab == "unlisted"} module={LivebookWeb.SessionLive.AddFileEntryUnlistedComponent} id="add-file-entry-from-unlisted" + hub={@hub} session={@session} file_entries={@file_entries} /> diff --git a/lib/livebook_web/live/session_live/add_file_entry_file_component.ex b/lib/livebook_web/live/session_live/add_file_entry_file_component.ex index 51d6a265b4a..222181d4ec4 100644 --- a/lib/livebook_web/live/session_live/add_file_entry_file_component.ex +++ b/lib/livebook_web/live/session_live/add_file_entry_file_component.ex @@ -63,6 +63,7 @@ defmodule LivebookWeb.SessionLive.AddFileEntryFileComponent do module={LivebookWeb.FileSelectComponent} id="add-file-entry-select" file={@file} + hub={@hub} extnames={:any} running_files={[]} target={{__MODULE__, @id}} From b9f3d93e763bedbe394232fea0800300b663a21c Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Wed, 13 Sep 2023 12:01:57 -0300 Subject: [PATCH 06/35] Handle hub assignment to list hub file systems --- lib/livebook_web/live/file_select_component.ex | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index eae07e3e2c8..3438b28cbbf 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -70,7 +70,11 @@ defmodule LivebookWeb.FileSelectComponent do |> assign(assigns) |> update_file_infos(force_reload? or running_files_changed?) - {:ok, socket} + if hub = socket.assigns[:hub] do + {:ok, assign(socket, file_systems: Livebook.Hubs.get_file_systems(hub))} + else + {:ok, assign(socket, file_systems: Livebook.Hubs.get_file_systems())} + end end @impl true From 70abc79f4e996dca39e4bf7045f14c929b0fbe77 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Thu, 10 Aug 2023 15:53:18 -0300 Subject: [PATCH 07/35] Add ID to menu item elements from file system --- lib/livebook_web/live/file_select_component.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index 3438b28cbbf..bf8aeba2f71 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -285,7 +285,7 @@ defmodule LivebookWeb.FileSelectComponent do <%= for file_system <- @file_systems do %> <%= if file_system == @file.file_system do %> <.menu_item variant={:selected}> - @@ -293,6 +293,7 @@ defmodule LivebookWeb.FileSelectComponent do <% else %> <.menu_item> + + <.menu_item> + <.link + id={"hub-file-system-#{file_system.id}-edit"} + patch={~p"/hub/#{@hub_id}/file-systems/edit/#{file_system.id}"} + type="button" + role="menuitem" + > + <.remix_icon icon="file-edit-line" /> + Edit + + + <.menu_item variant={:danger}> + + + + + + +
+ <.link + patch={~p"/hub/#{@hub_id}/file-systems/new"} + class="button-base button-blue" + id="add-file-system" + > + Add file storage + +
+ + """ + end + + @impl true + def handle_event("delete_file_system", %{"id" => id, "name" => name}, socket) do + on_confirm = fn socket -> + hub = Livebook.Hubs.fetch_hub!(socket.assigns.hub.id) + file_systems = Livebook.Hubs.get_file_systems(hub) + file_system = Enum.find(file_systems, &(&1.id == id)) + + case Livebook.Hubs.delete_file_system(hub, file_system) do + :ok -> socket + {:transport_error, reason} -> put_flash(socket, :error, reason) + end + end + + {:noreply, + confirm(socket, on_confirm, + title: "Delete hub file system - #{name}", + description: "Are you sure you want to delete this hub file system?", + confirm_text: "Delete", + confirm_icon: "delete-bin-6-line" + )} + end + + defp type(file_system), do: file_system |> FileSystems.type() |> String.upcase() + defp name(file_system), do: file_system |> FileSystem.external_metadata() |> Map.get(:name) +end From 66591dd1394b458e05b092a2e0a5985d74cadd79 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 15 Sep 2023 16:17:30 -0300 Subject: [PATCH 16/35] Add FileSystemFormComponent --- .../live/hub/file_system_form_component.ex | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 lib/livebook_web/live/hub/file_system_form_component.ex diff --git a/lib/livebook_web/live/hub/file_system_form_component.ex b/lib/livebook_web/live/hub/file_system_form_component.ex new file mode 100644 index 00000000000..057bc4e79b8 --- /dev/null +++ b/lib/livebook_web/live/hub/file_system_form_component.ex @@ -0,0 +1,118 @@ +defmodule LivebookWeb.Hub.FileSystemFormComponent do + use LivebookWeb, :live_component + + alias Livebook.FileSystem + alias Livebook.FileSystems + + @impl true + def update(%{file_system: file_system} = assigns, socket) do + file_system = file_system || %FileSystem.S3{} + changeset = FileSystems.change_file_system(file_system) + socket = assign(socket, assigns) + + {:ok, + assign(socket, + file_system: file_system, + changeset: changeset, + mode: mode(socket), + title: title(socket), + button: button(socket), + error_message: nil + )} + end + + @impl true + def render(assigns) do + ~H""" +
+

+ <%= @title %> +

+

+ Configure an AWS S3 bucket as a Livebook file system. + Many storage services offer an S3-compatible API and + those work as well. +

+
+ <%= @error_message %> +
+ <.form + :let={f} + id="file-systems-form" + for={to_form(@changeset, as: :file_system)} + phx-target={@myself} + phx-submit="save" + phx-change="validate" + autocomplete="off" + spellcheck="false" + > +
+ <.text_field + field={f[:bucket_url]} + label="Bucket URL" + placeholder="https://s3.[region].amazonaws.com/[bucket]" + /> + <.text_field field={f[:region]} label="Region (optional)" /> + <.password_field field={f[:access_key_id]} label="Access Key ID" /> + <.password_field field={f[:secret_access_key]} label="Secret Access Key" /> +
+ + <.link patch={@return_to} class="button-base button-outlined-gray"> + Cancel + +
+
+ +
+ """ + end + + @impl true + def handle_event("validate", %{"file_system" => attrs}, socket) do + changeset = + socket.assigns.file_system + |> FileSystems.change_file_system(attrs) + |> Map.replace!(:action, :validate) + + {:noreply, assign(socket, changeset: changeset)} + end + + def handle_event("save", %{"file_system" => attrs}, socket) do + with {:ok, file_system} <- FileSystems.update_file_system(socket.assigns.file_system, attrs), + :ok <- check_file_system_conectivity(file_system), + :ok <- save_file_system(file_system, socket) do + {:noreply, push_patch(socket, to: socket.assigns.return_to)} + else + {:error, %Ecto.Changeset{} = changeset} -> {:noreply, assign(socket, changeset: changeset)} + {:error, message} -> {:noreply, assign(socket, error_message: message)} + end + end + + defp check_file_system_conectivity(file_system) do + default_dir = FileSystem.File.new(file_system) + + case FileSystem.File.list(default_dir) do + {:ok, _} -> :ok + {:error, message} -> {:error, "Connection test failed: " <> message} + end + end + + defp save_file_system(file_system, socket) do + case socket.assigns.mode do + :new -> Livebook.Hubs.create_file_system(socket.assigns.hub, file_system) + :edit -> Livebook.Hubs.update_file_system(socket.assigns.hub, file_system) + end + end + + defp mode(%{assigns: %{file_system: nil}}), do: :new + defp mode(_), do: :edit + + defp title(%{assigns: %{file_system: nil}}), do: "Add file storage" + defp title(_), do: "Edit file storage" + + defp button(%{assigns: %{file_system: nil}}), do: %{icon: "add-line", label: "Add"} + defp button(_), do: %{icon: "save-line", label: "Save"} +end From a87afbae6278d0e205e2e6778bc06b538bb1467a Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 15 Sep 2023 16:17:39 -0300 Subject: [PATCH 17/35] Handle file system events --- lib/livebook_web/live/hub/edit_live.ex | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/livebook_web/live/hub/edit_live.ex b/lib/livebook_web/live/hub/edit_live.ex index d280b6a9252..094482b2bbd 100644 --- a/lib/livebook_web/live/hub/edit_live.ex +++ b/lib/livebook_web/live/hub/edit_live.ex @@ -14,7 +14,7 @@ defmodule LivebookWeb.Hub.EditLive do @impl true def handle_params(params, _url, socket) do - Hubs.subscribe([:connection, :secrets]) + Hubs.subscribe([:connection, :secrets, :file_systems]) hub = Hubs.fetch_hub!(params["id"]) type = Provider.type(hub) @@ -115,6 +115,27 @@ defmodule LivebookWeb.Hub.EditLive do |> put_flash(:success, "Secret #{name} deleted successfully")} end + def handle_info({:file_system_created, _}, socket) do + {:noreply, + socket + |> push_navigate(to: ~p"/hub/#{socket.assigns.hub.id}") + |> put_flash(:success, "File storage created successfully")} + end + + def handle_info({:file_system_updated, _}, socket) do + {:noreply, + socket + |> push_navigate(to: ~p"/hub/#{socket.assigns.hub.id}") + |> put_flash(:success, "File storage updated successfully")} + end + + def handle_info({:file_system_deleted, _}, socket) do + {:noreply, + socket + |> push_navigate(to: ~p"/hub/#{socket.assigns.hub.id}") + |> put_flash(:success, "File storage deleted successfully")} + end + def handle_info({:hub_connected, id}, %{assigns: %{hub: %{id: id}}} = socket) do {:noreply, push_navigate(socket, to: ~p"/hub/#{id}")} end From 369324702b891813627184917652a1978752cf3c Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 15 Sep 2023 16:17:55 -0300 Subject: [PATCH 18/35] Implement file system components --- .../live/hub/edit/personal_component.ex | 46 +++++++++++++++++++ .../live/hub/edit/team_component.ex | 46 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/lib/livebook_web/live/hub/edit/personal_component.ex b/lib/livebook_web/live/hub/edit/personal_component.ex index 330c0728675..5068b11aac5 100644 --- a/lib/livebook_web/live/hub/edit/personal_component.ex +++ b/lib/livebook_web/live/hub/edit/personal_component.ex @@ -11,7 +11,9 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do socket = assign(socket, assigns) changeset = Personal.change_hub(assigns.hub) secrets = Hubs.get_secrets(assigns.hub) + [_local | file_systems] = Hubs.get_file_systems(assigns.hub) secret_name = assigns.params["secret_name"] + file_system_id = assigns.params["file_system_id"] secret_value = if assigns.live_action == :edit_secret do @@ -19,9 +21,18 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do raise(NotFoundError, "could not find secret matching #{inspect(secret_name)}") end + file_system = + if assigns.live_action == :edit_file_system do + Enum.find_value(file_systems, &(&1.id == file_system_id && &1)) || + raise(NotFoundError, "could not find file system matching #{inspect(file_system_id)}") + end + {:ok, assign(socket, secrets: secrets, + file_system: file_system, + file_system_id: file_system_id, + file_systems: file_systems, changeset: changeset, stamp_changeset: changeset, secret_name: secret_name, @@ -94,6 +105,24 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do /> +
+

+ File Storages +

+ +

+ File storages are used to store notebooks. +

+ + <.live_component + module={LivebookWeb.Hub.FileSystemListComponent} + id="hub-file-systems-list" + hub_id={@hub.id} + file_systems={@file_systems} + target={@myself} + /> +
+

Stamping @@ -167,6 +196,23 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do return_to={~p"/hub/#{@hub.id}"} /> + + <.modal + :if={@live_action in [:new_file_system, :edit_file_system]} + id="file-systems-modal" + show + width={:medium} + patch={~p"/hub/#{@hub.id}"} + > + <.live_component + module={LivebookWeb.Hub.FileSystemFormComponent} + id="file-systems" + hub={@hub} + file_system={@file_system} + file_system_id={@file_system_id} + return_to={~p"/hub/#{@hub.id}"} + /> +

""" end diff --git a/lib/livebook_web/live/hub/edit/team_component.ex b/lib/livebook_web/live/hub/edit/team_component.ex index 3e7b4fe7a77..51f0e7e6524 100644 --- a/lib/livebook_web/live/hub/edit/team_component.ex +++ b/lib/livebook_web/live/hub/edit/team_component.ex @@ -13,7 +13,9 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do changeset = Team.change_hub(assigns.hub) show_key? = assigns.params["show-key"] == "true" secrets = Livebook.Hubs.get_secrets(assigns.hub) + [_local | file_systems] = Hubs.get_file_systems(assigns.hub) secret_name = assigns.params["secret_name"] + file_system_id = assigns.params["file_system_id"] is_default? = is_default?(assigns.hub) secret_value = @@ -22,10 +24,19 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do raise(NotFoundError, "could not find secret matching #{inspect(secret_name)}") end + file_system = + if assigns.live_action == :edit_file_system do + Enum.find_value(file_systems, &(&1.id == file_system_id)) || + raise(NotFoundError, "could not find file system matching #{inspect(file_system_id)}") + end + {:ok, socket |> assign( secrets: secrets, + file_system: file_system, + file_system_id: file_system_id, + file_systems: file_systems, show_key: show_key?, secret_name: secret_name, secret_value: secret_value, @@ -163,6 +174,24 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do /> +
+

+ File Storages +

+ +

+ File storages are used to store notebooks. +

+ + <.live_component + module={LivebookWeb.Hub.FileSystemListComponent} + id="hub-file-systems-list" + hub_id={@hub.id} + file_systems={@file_systems} + target={@myself} + /> +
+

Airgapped Deployment @@ -311,6 +340,23 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do return_to={~p"/hub/#{@hub.id}"} /> + + <.modal + :if={@live_action in [:new_file_system, :edit_file_system]} + id="file-systems-modal" + show + width={:medium} + patch={~p"/hub/#{@hub.id}"} + > + <.live_component + module={LivebookWeb.Hub.FileSystemFormComponent} + id="file-systems" + hub={@hub} + file_system={@file_system} + file_system_id={@file_system_id} + return_to={~p"/hub/#{@hub.id}"} + /> +

From 8d5edcf713f742c7b55ac95298bac5ffba2f6618 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 15 Sep 2023 16:18:06 -0300 Subject: [PATCH 19/35] Add functions to help handling file system forms --- lib/livebook/file_systems.ex | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/livebook/file_systems.ex b/lib/livebook/file_systems.ex index c50b86d137a..1a7efb05925 100644 --- a/lib/livebook/file_systems.ex +++ b/lib/livebook/file_systems.ex @@ -9,6 +9,33 @@ defmodule Livebook.FileSystems do @spec type(FileSystem.t()) :: String.t() def type(%FileSystem.S3{}), do: "s3" + @doc """ + Updates file system with the given changes. + """ + @spec update_file_system(FileSystem.t(), map()) :: + {:ok, FileSystem.t()} | {:error, Ecto.Changeset.t()} + def update_file_system(file_system, attrs) do + file_system + |> change_file_system(attrs) + |> Ecto.Changeset.apply_action(:update) + end + + @doc """ + Returns an `%Ecto.Changeset{}` for tracking file system changes. + """ + @spec change_file_system(FileSystem.t()) :: Ecto.Changeset.t() + def change_file_system(file_system) do + change_file_system(file_system, %{}) + end + + @doc """ + Returns an `%Ecto.Changeset{}` for tracking file system changes. + """ + @spec change_file_system(FileSystem.t(), map()) :: Ecto.Changeset.t() + def change_file_system(%FileSystem.S3{} = file_system, attrs) do + FileSystem.S3.change_file_system(file_system, attrs) + end + @doc """ Loads the file system from given type and dumped data. """ From dc8e8745b1195473383b5988b954e430459803b1 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 15 Sep 2023 16:18:13 -0300 Subject: [PATCH 20/35] Add tests --- .../live/hub/edit/team_component.ex | 2 +- .../livebook_teams/web/hub/edit_live_test.exs | 136 +++++++++++++++++- test/livebook_web/live/hub/edit_live_test.exs | 130 ++++++++++++++++- test/support/hub_helpers.ex | 13 ++ 4 files changed, 278 insertions(+), 3 deletions(-) diff --git a/lib/livebook_web/live/hub/edit/team_component.ex b/lib/livebook_web/live/hub/edit/team_component.ex index 51f0e7e6524..c565570af81 100644 --- a/lib/livebook_web/live/hub/edit/team_component.ex +++ b/lib/livebook_web/live/hub/edit/team_component.ex @@ -26,7 +26,7 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do file_system = if assigns.live_action == :edit_file_system do - Enum.find_value(file_systems, &(&1.id == file_system_id)) || + Enum.find_value(file_systems, &(&1.id == file_system_id && &1)) || raise(NotFoundError, "could not find file system matching #{inspect(file_system_id)}") end diff --git a/test/livebook_teams/web/hub/edit_live_test.exs b/test/livebook_teams/web/hub/edit_live_test.exs index 28e7f8825d1..7ccadf4eab4 100644 --- a/test/livebook_teams/web/hub/edit_live_test.exs +++ b/test/livebook_teams/web/hub/edit_live_test.exs @@ -7,7 +7,7 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do alias Livebook.Hubs setup %{user: user, node: node} do - Livebook.Hubs.subscribe([:crud, :connection, :secrets]) + Livebook.Hubs.subscribe([:crud, :connection, :secrets, :file_systems]) hub = create_team_hub(user, node) id = hub.id @@ -252,5 +252,139 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do |> element("span", "Default") |> has_element?() end + + test "creates a file system", %{conn: conn, hub: hub} do + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + bypass = Bypass.open() + file_system = build_bypass_file_system(bypass) + id = "#{hub.id}-#{file_system.id}" + attrs = %{file_system: Livebook.FileSystem.dump(file_system)} + + Bypass.expect_once(bypass, "GET", "/", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + refute render(view) =~ file_system.bucket_url + + view + |> element("#add-file-system") + |> render_click(%{}) + + assert_patch(view, ~p"/hub/#{hub.id}/file-systems/new") + assert render(view) =~ "Add file storage" + + view + |> element("#file-systems-form") + |> render_change(attrs) + + refute view + |> element("#file-systems-form button[disabled]") + |> has_element?() + + view + |> element("#file-systems-form") + |> render_submit(attrs) + + assert_receive {:file_system_created, %{id: ^id} = file_system} + + %{"success" => "File storage created successfully"} = + assert_redirect(view, "/hub/#{hub.id}") + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + assert render(element(view, "#hub-file-systems-list")) =~ file_system.bucket_url + assert file_system in Livebook.Hubs.get_file_systems(hub) + end + + test "updates existing file system", %{conn: conn, hub: hub} do + bypass = Bypass.open() + file_system = build_bypass_file_system(bypass) + id = "#{hub.id}-#{file_system.id}" + + :ok = Hubs.create_file_system(hub, file_system) + assert_receive {:file_system_created, %{id: ^id} = file_system} + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + attrs = %{file_system: Livebook.FileSystem.dump(file_system)} + + Bypass.expect_once(bypass, "GET", "/", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + view + |> element("#hub-file-system-#{file_system.id}-edit") + |> render_click(%{"file_system" => file_system}) + + assert_patch(view, ~p"/hub/#{hub.id}/file-systems/edit/#{file_system.id}") + assert render(view) =~ "Edit file storage" + + view + |> element("#file-systems-form") + |> render_change(attrs) + + refute view + |> element("#file-systems-form button[disabled]") + |> has_element?() + + view + |> element("#file-systems-form") + |> render_submit(put_in(attrs.file_system.access_key_id, "new key")) + + updated_file_system = %{file_system | access_key_id: "new key"} + assert_receive {:file_system_updated, ^updated_file_system} + + %{"success" => "File storage updated successfully"} = + assert_redirect(view, "/hub/#{hub.id}") + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + assert render(element(view, "#hub-file-systems-list")) =~ file_system.bucket_url + assert updated_file_system in Livebook.Hubs.get_file_systems(hub) + end + + test "deletes existing file system", %{conn: conn, hub: hub} do + bypass = Bypass.open() + file_system = build_bypass_file_system(bypass) + id = "#{hub.id}-#{file_system.id}" + + :ok = Hubs.create_file_system(hub, file_system) + assert_receive {:file_system_created, %{id: ^id} = file_system} + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + refute view + |> element("#file-systems-form button[disabled]") + |> has_element?() + + view + |> element("#hub-file-system-#{file_system.id}-delete", "Delete") + |> render_click() + + render_confirm(view) + + assert_receive {:file_system_deleted, ^file_system} + + %{"success" => "File storage deleted successfully"} = + assert_redirect(view, "/hub/#{hub.id}") + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + refute render(element(view, "#hub-file-systems-list")) =~ file_system.bucket_url + refute file_system in Livebook.Hubs.get_file_systems(hub) + end end end diff --git a/test/livebook_web/live/hub/edit_live_test.exs b/test/livebook_web/live/hub/edit_live_test.exs index 5e3d91e192c..b7cd24ea6cf 100644 --- a/test/livebook_web/live/hub/edit_live_test.exs +++ b/test/livebook_web/live/hub/edit_live_test.exs @@ -9,7 +9,7 @@ defmodule LivebookWeb.Hub.EditLiveTest do describe "personal" do setup do - Livebook.Hubs.subscribe([:crud, :secrets]) + Livebook.Hubs.subscribe([:crud, :secrets, :file_systems]) {:ok, hub: Hubs.fetch_hub!(Hubs.Personal.id())} end @@ -161,5 +161,133 @@ defmodule LivebookWeb.Hub.EditLiveTest do refute render(element(view, "#hub-secrets-list")) =~ secret.name refute secret in Livebook.Hubs.get_secrets(hub) end + + test "creates file system", %{conn: conn, hub: hub} do + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + bypass = Bypass.open() + file_system = build_bypass_file_system(bypass) + + attrs = %{file_system: Livebook.FileSystem.dump(file_system)} + + Bypass.expect_once(bypass, "GET", "/", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + refute render(view) =~ file_system.bucket_url + + view + |> element("#add-file-system") + |> render_click(%{}) + + assert_patch(view, ~p"/hub/#{hub.id}/file-systems/new") + assert render(view) =~ "Add file storage" + + view + |> element("#file-systems-form") + |> render_change(attrs) + + refute view + |> element("#file-systems-form button[disabled]") + |> has_element?() + + view + |> element("#file-systems-form") + |> render_submit(attrs) + + assert_receive {:file_system_created, ^file_system} + + %{"success" => "File storage created successfully"} = + assert_redirect(view, "/hub/#{hub.id}") + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + assert render(element(view, "#hub-file-systems-list")) =~ file_system.bucket_url + assert file_system in Livebook.Hubs.get_file_systems(hub) + end + + test "updates file system", %{conn: conn, hub: hub} do + bypass = Bypass.open() + file_system = build_bypass_file_system(bypass) + :ok = Hubs.create_file_system(hub, file_system) + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + attrs = %{file_system: Livebook.FileSystem.dump(file_system)} + + Bypass.expect_once(bypass, "GET", "/", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + + view + |> element("#hub-file-system-#{file_system.id}-edit") + |> render_click(%{"file_system" => file_system}) + + assert_patch(view, ~p"/hub/#{hub.id}/file-systems/edit/#{file_system.id}") + assert render(view) =~ "Edit file storage" + + view + |> element("#file-systems-form") + |> render_change(attrs) + + refute view + |> element("#file-systems-form button[disabled]") + |> has_element?() + + view + |> element("#file-systems-form") + |> render_submit(put_in(attrs.file_system.access_key_id, "new key")) + + updated_file_system = %{file_system | access_key_id: "new key"} + assert_receive {:file_system_updated, ^updated_file_system} + + %{"success" => "File storage updated successfully"} = + assert_redirect(view, "/hub/#{hub.id}") + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + assert render(element(view, "#hub-file-systems-list")) =~ file_system.bucket_url + assert updated_file_system in Livebook.Hubs.get_file_systems(hub) + end + + test "deletes file system", %{conn: conn, hub: hub} do + bypass = Bypass.open() + file_system = build_bypass_file_system(bypass) + :ok = Hubs.create_file_system(hub, file_system) + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + refute view + |> element("#file-systems-form button[disabled]") + |> has_element?() + + view + |> element("#hub-file-system-#{file_system.id}-delete", "Delete") + |> render_click() + + render_confirm(view) + + assert_receive {:file_system_deleted, ^file_system} + + %{"success" => "File storage deleted successfully"} = + assert_redirect(view, "/hub/#{hub.id}") + + {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") + + refute render(element(view, "#hub-file-systems-list")) =~ file_system.bucket_url + refute file_system in Livebook.Hubs.get_file_systems(hub) + end end end diff --git a/test/support/hub_helpers.ex b/test/support/hub_helpers.ex index 303dac0c0b7..a872100936f 100644 --- a/test/support/hub_helpers.ex +++ b/test/support/hub_helpers.ex @@ -108,6 +108,19 @@ defmodule Livebook.HubHelpers do :erpc.call(node, Hub.Integration, :create_file_system, [[org_key: org_key]]) end + def build_bypass_file_system(bypass) do + bucket_url = "http://localhost:#{bypass.port}" + hash = :crypto.hash(:sha256, bucket_url) + + file_system = + build(:fs_s3, + id: "s3-#{Base.url_encode64(hash, padding: false)}", + bucket_url: bucket_url + ) + + file_system + end + defp hub_pid(hub) do if pid = GenServer.whereis({:via, Registry, {Livebook.HubsRegistry, hub.id}}) do {:ok, pid} From 9251981c8d515ba07d8fd5222285f914458db290 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Mon, 18 Sep 2023 12:06:23 -0300 Subject: [PATCH 21/35] Change flash message from `created` to `added` --- lib/livebook_web/live/hub/edit_live.ex | 4 ++-- lib/livebook_web/live/hub/secret_form_component.ex | 2 +- test/livebook_teams/web/hub/edit_live_test.exs | 4 ++-- test/livebook_web/live/hub/edit_live_test.exs | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/livebook_web/live/hub/edit_live.ex b/lib/livebook_web/live/hub/edit_live.ex index 094482b2bbd..25f59504c6b 100644 --- a/lib/livebook_web/live/hub/edit_live.ex +++ b/lib/livebook_web/live/hub/edit_live.ex @@ -92,7 +92,7 @@ defmodule LivebookWeb.Hub.EditLive do {:noreply, socket |> push_navigate(to: ~p"/hub/#{id}") - |> put_flash(:success, "Secret #{name} created successfully")} + |> put_flash(:success, "Secret #{name} added successfully")} end def handle_info( @@ -119,7 +119,7 @@ defmodule LivebookWeb.Hub.EditLive do {:noreply, socket |> push_navigate(to: ~p"/hub/#{socket.assigns.hub.id}") - |> put_flash(:success, "File storage created successfully")} + |> put_flash(:success, "File storage added successfully")} end def handle_info({:file_system_updated, _}, socket) do diff --git a/lib/livebook_web/live/hub/secret_form_component.ex b/lib/livebook_web/live/hub/secret_form_component.ex index 51d7478ad12..279f1aa6a00 100644 --- a/lib/livebook_web/live/hub/secret_form_component.ex +++ b/lib/livebook_web/live/hub/secret_form_component.ex @@ -80,7 +80,7 @@ defmodule LivebookWeb.Hub.SecretFormComponent do message = if socket.assigns.secret_name, do: "Secret #{secret.name} updated successfully", - else: "Secret #{secret.name} created successfully" + else: "Secret #{secret.name} added successfully" {:noreply, socket diff --git a/test/livebook_teams/web/hub/edit_live_test.exs b/test/livebook_teams/web/hub/edit_live_test.exs index 7ccadf4eab4..97f1f7221b2 100644 --- a/test/livebook_teams/web/hub/edit_live_test.exs +++ b/test/livebook_teams/web/hub/edit_live_test.exs @@ -99,7 +99,7 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do assert_receive {:secret_created, ^secret} - %{"success" => "Secret TEAM_ADD_SECRET created successfully"} = + %{"success" => "Secret TEAM_ADD_SECRET added successfully"} = assert_redirect(view, "/hub/#{hub.id}") {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") @@ -294,7 +294,7 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do assert_receive {:file_system_created, %{id: ^id} = file_system} - %{"success" => "File storage created successfully"} = + %{"success" => "File storage added successfully"} = assert_redirect(view, "/hub/#{hub.id}") {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") diff --git a/test/livebook_web/live/hub/edit_live_test.exs b/test/livebook_web/live/hub/edit_live_test.exs index b7cd24ea6cf..15d58358c0b 100644 --- a/test/livebook_web/live/hub/edit_live_test.exs +++ b/test/livebook_web/live/hub/edit_live_test.exs @@ -82,7 +82,7 @@ defmodule LivebookWeb.Hub.EditLiveTest do assert_receive {:secret_created, ^secret} - %{"success" => "Secret PERSONAL_ADD_SECRET created successfully"} = + %{"success" => "Secret PERSONAL_ADD_SECRET added successfully"} = assert_redirect(view, "/hub/#{hub.id}") {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") @@ -203,7 +203,7 @@ defmodule LivebookWeb.Hub.EditLiveTest do assert_receive {:file_system_created, ^file_system} - %{"success" => "File storage created successfully"} = + %{"success" => "File storage added successfully"} = assert_redirect(view, "/hub/#{hub.id}") {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") From db477ba3d3f5db4f3f6fb97afdbd77aa7dffec1f Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Mon, 18 Sep 2023 12:06:41 -0300 Subject: [PATCH 22/35] Improve confirm dialog when deleting a file system --- lib/livebook_web/live/hub/file_system_list_component.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/livebook_web/live/hub/file_system_list_component.ex b/lib/livebook_web/live/hub/file_system_list_component.ex index 5f74be668b4..5fd60ffc9d1 100644 --- a/lib/livebook_web/live/hub/file_system_list_component.ex +++ b/lib/livebook_web/live/hub/file_system_list_component.ex @@ -87,8 +87,8 @@ defmodule LivebookWeb.Hub.FileSystemListComponent do {:noreply, confirm(socket, on_confirm, - title: "Delete hub file system - #{name}", - description: "Are you sure you want to delete this hub file system?", + title: "Delete hub file storage", + description: "Are you sure you want to delete #{name}?", confirm_text: "Delete", confirm_icon: "delete-bin-6-line" )} From 87edef6a417d6e1555929c186fdfcd6e017bb588 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Mon, 18 Sep 2023 12:07:17 -0300 Subject: [PATCH 23/35] Use file systems from notebook's hub --- lib/livebook_web/live/session_live.ex | 1 + lib/livebook_web/live/session_live/persistence_component.ex | 2 ++ 2 files changed, 3 insertions(+) diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 224363ddde3..07e875e0396 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -461,6 +461,7 @@ defmodule LivebookWeb.SessionLive do id="persistence" session={@session} file={@data_view.file} + hub={@data_view.hub} persist_outputs={@data_view.persist_outputs} autosave_interval_s={@data_view.autosave_interval_s} /> diff --git a/lib/livebook_web/live/session_live/persistence_component.ex b/lib/livebook_web/live/session_live/persistence_component.ex index c04f3f06c4a..6a3a09f548b 100644 --- a/lib/livebook_web/live/session_live/persistence_component.ex +++ b/lib/livebook_web/live/session_live/persistence_component.ex @@ -49,6 +49,7 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do socket = socket |> assign(assigns) + |> assign_new(:hub, fn -> nil end) |> assign_new(:attrs, fn -> attrs end) |> assign_new(:new_attrs, fn -> attrs end) |> assign_new(:draft_file, fn -> @@ -77,6 +78,7 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do module={LivebookWeb.FileSelectComponent} id="persistence_file_select" file={@draft_file} + hub={@hub} extnames={[LiveMarkdown.extension()]} running_files={@running_files} submit_event={:confirm_file} From fc916b6b572c1ae6266356908a77e1c2febda9e1 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Mon, 18 Sep 2023 12:28:21 -0300 Subject: [PATCH 24/35] Use configure path based on current hub --- lib/livebook_web/live/file_select_component.ex | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index bf8aeba2f71..9b39a4beb72 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -45,6 +45,7 @@ defmodule LivebookWeb.FileSelectComponent do renaming_file: nil, renamed_name: nil, error_message: nil, + configure_path: nil, file_systems: [] ) |> allow_upload(:folder, @@ -70,11 +71,14 @@ defmodule LivebookWeb.FileSelectComponent do |> assign(assigns) |> update_file_infos(force_reload? or running_files_changed?) - if hub = socket.assigns[:hub] do - {:ok, assign(socket, file_systems: Livebook.Hubs.get_file_systems(hub))} - else - {:ok, assign(socket, file_systems: Livebook.Hubs.get_file_systems())} - end + configure_path = ~p"/hub/#{Livebook.Hubs.Personal.id()}/file-systems/new" + + {file_systems, configure_path} = + if hub = socket.assigns[:hub], + do: {Livebook.Hubs.get_file_systems(hub), ~p"/hub/#{hub.id}/file-systems/new"}, + else: {Livebook.Hubs.get_file_systems(), configure_path} + + {:ok, assign(socket, file_systems: file_systems, configure_path: configure_path)} end @impl true @@ -87,6 +91,7 @@ defmodule LivebookWeb.FileSelectComponent do <.file_system_menu_button file={@file} file_systems={@file_systems} + configure_path={@configure_path} file_system_select_disabled={@file_system_select_disabled} myself={@myself} /> @@ -306,7 +311,7 @@ defmodule LivebookWeb.FileSelectComponent do <% end %> <% end %> <.menu_item> - <.link navigate={~p"/settings"} class="border-t border-gray-200" role="menuitem"> + <.link navigate={@configure_path} class="border-t border-gray-200" role="menuitem"> <.remix_icon icon="settings-3-line" /> Configure From 32cfbe91ddc0613dda98197ec85b0a7f6e241616 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Mon, 18 Sep 2023 12:34:56 -0300 Subject: [PATCH 25/35] Rename from `Delete` to `Detach` --- .../live/hub/file_system_list_component.ex | 14 +++++++------- test/livebook_teams/web/hub/edit_live_test.exs | 4 ++-- test/livebook_web/live/hub/edit_live_test.exs | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/livebook_web/live/hub/file_system_list_component.ex b/lib/livebook_web/live/hub/file_system_list_component.ex index 5fd60ffc9d1..fb669ccfa14 100644 --- a/lib/livebook_web/live/hub/file_system_list_component.ex +++ b/lib/livebook_web/live/hub/file_system_list_component.ex @@ -40,19 +40,19 @@ defmodule LivebookWeb.Hub.FileSystemListComponent do <.menu_item variant={:danger}> @@ -73,7 +73,7 @@ defmodule LivebookWeb.Hub.FileSystemListComponent do end @impl true - def handle_event("delete_file_system", %{"id" => id, "name" => name}, socket) do + def handle_event("detach_file_system", %{"id" => id, "name" => name}, socket) do on_confirm = fn socket -> hub = Livebook.Hubs.fetch_hub!(socket.assigns.hub.id) file_systems = Livebook.Hubs.get_file_systems(hub) @@ -87,9 +87,9 @@ defmodule LivebookWeb.Hub.FileSystemListComponent do {:noreply, confirm(socket, on_confirm, - title: "Delete hub file storage", - description: "Are you sure you want to delete #{name}?", - confirm_text: "Delete", + title: "Detach hub file storage", + description: "Are you sure you want to detach #{name}?", + confirm_text: "Detach", confirm_icon: "delete-bin-6-line" )} end diff --git a/test/livebook_teams/web/hub/edit_live_test.exs b/test/livebook_teams/web/hub/edit_live_test.exs index 97f1f7221b2..ab66f7b5c0d 100644 --- a/test/livebook_teams/web/hub/edit_live_test.exs +++ b/test/livebook_teams/web/hub/edit_live_test.exs @@ -356,7 +356,7 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do assert updated_file_system in Livebook.Hubs.get_file_systems(hub) end - test "deletes existing file system", %{conn: conn, hub: hub} do + test "detaches existing file system", %{conn: conn, hub: hub} do bypass = Bypass.open() file_system = build_bypass_file_system(bypass) id = "#{hub.id}-#{file_system.id}" @@ -371,7 +371,7 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do |> has_element?() view - |> element("#hub-file-system-#{file_system.id}-delete", "Delete") + |> element("#hub-file-system-#{file_system.id}-detach", "Detach") |> render_click() render_confirm(view) diff --git a/test/livebook_web/live/hub/edit_live_test.exs b/test/livebook_web/live/hub/edit_live_test.exs index 15d58358c0b..e95463691e5 100644 --- a/test/livebook_web/live/hub/edit_live_test.exs +++ b/test/livebook_web/live/hub/edit_live_test.exs @@ -262,7 +262,7 @@ defmodule LivebookWeb.Hub.EditLiveTest do assert updated_file_system in Livebook.Hubs.get_file_systems(hub) end - test "deletes file system", %{conn: conn, hub: hub} do + test "detaches file system", %{conn: conn, hub: hub} do bypass = Bypass.open() file_system = build_bypass_file_system(bypass) :ok = Hubs.create_file_system(hub, file_system) @@ -274,7 +274,7 @@ defmodule LivebookWeb.Hub.EditLiveTest do |> has_element?() view - |> element("#hub-file-system-#{file_system.id}-delete", "Delete") + |> element("#hub-file-system-#{file_system.id}-detach", "Detach") |> render_click() render_confirm(view) From 2ec8508253b5110831e7255f041af175b3d34c5b Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Mon, 18 Sep 2023 14:51:13 -0300 Subject: [PATCH 26/35] Fix specs --- lib/livebook/file_system/s3.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index a3c3a5386bc..f9d1e1c345c 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -8,7 +8,7 @@ defmodule Livebook.FileSystem.S3 do @type t :: %__MODULE__{ id: String.t(), bucket_url: String.t(), - external_id: String.t(), + external_id: String.t() | nil, region: String.t(), access_key_id: String.t(), secret_access_key: String.t() From 8a6415664629ce7e447bbfb54e9d8d028ce8790f Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Mon, 18 Sep 2023 16:34:48 -0300 Subject: [PATCH 27/35] Put region on changeset --- lib/livebook/file_system/s3.ex | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index f9d1e1c345c..2d4b16cac5c 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -63,7 +63,13 @@ defmodule Livebook.FileSystem.S3 do defp region_from_uri(uri) do # For many services the API host is of the form *.[region].[rootdomain].com %{host: host} = URI.parse(uri) - host |> String.split(".") |> Enum.reverse() |> Enum.at(2, "auto") + splitted_host = host |> String.split(".") |> Enum.reverse() + + case Enum.at(splitted_host, 2) do + "s3" -> "us-east-1" + "r2" -> "auto" + region -> region + end end @doc """ @@ -113,11 +119,19 @@ defmodule Livebook.FileSystem.S3 do :access_key_id, :secret_access_key ]) + |> put_region_from_uri() |> validate_required([:bucket_url, :access_key_id, :secret_access_key]) |> Livebook.Utils.validate_url(:bucket_url) |> put_id() end + defp put_region_from_uri(changeset) do + case get_field(changeset, :bucket_url) do + nil -> changeset + bucket_url -> put_change(changeset, :region, region_from_uri(bucket_url)) + end + end + defp put_id(changeset) do if bucket_url = get_field(changeset, :bucket_url) do hash = :crypto.hash(:sha256, bucket_url) From 0f30bdf1eadc87aa170f30f848e3a233c21afa9e Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Mon, 18 Sep 2023 16:55:44 -0300 Subject: [PATCH 28/35] Fix tests --- lib/livebook/file_system/s3.ex | 2 +- test/support/hub_helpers.ex | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index 2d4b16cac5c..722de9eb132 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -65,7 +65,7 @@ defmodule Livebook.FileSystem.S3 do %{host: host} = URI.parse(uri) splitted_host = host |> String.split(".") |> Enum.reverse() - case Enum.at(splitted_host, 2) do + case Enum.at(splitted_host, 2, "auto") do "s3" -> "us-east-1" "r2" -> "auto" region -> region diff --git a/test/support/hub_helpers.ex b/test/support/hub_helpers.ex index a872100936f..c84e4a66c79 100644 --- a/test/support/hub_helpers.ex +++ b/test/support/hub_helpers.ex @@ -115,7 +115,8 @@ defmodule Livebook.HubHelpers do file_system = build(:fs_s3, id: "s3-#{Base.url_encode64(hash, padding: false)}", - bucket_url: bucket_url + bucket_url: bucket_url, + region: "auto" ) file_system From ff2734d0c536804013b6371732550e85a6f1d983 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 22 Sep 2023 14:42:46 -0300 Subject: [PATCH 29/35] Apply review comments --- lib/livebook/file_system/s3.ex | 18 ++++--- lib/livebook/hubs.ex | 16 +++--- lib/livebook/session.ex | 21 +++++--- .../live/file_select_component.ex | 10 ++-- .../live/hub/edit/personal_component.ex | 23 +------- .../live/hub/edit/team_component.ex | 2 +- lib/livebook_web/live/hub/edit_live.ex | 54 ++----------------- .../live/hub/file_system_form_component.ex | 32 ++++++++--- .../live/hub/file_system_list_component.ex | 8 ++- .../live/hub/secret_list_component.ex | 10 ++-- .../session_live/persistence_component.ex | 1 - test/livebook_web/live/hub/edit_live_test.exs | 32 +++++------ 12 files changed, 94 insertions(+), 133 deletions(-) diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index 722de9eb132..ae86267172d 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -43,12 +43,10 @@ defmodule Livebook.FileSystem.S3 do bucket_url = String.trim_trailing(bucket_url, "/") region = opts[:region] || region_from_uri(bucket_url) - hash = :crypto.hash(:sha256, bucket_url) |> Base.url_encode64(padding: false) - id = if prefix = opts[:prefix], - do: "#{prefix}-s3-#{hash}", - else: "s3-#{hash}" + do: "#{prefix}-#{id(bucket_url)}", + else: id(bucket_url) %__MODULE__{ id: id, @@ -134,14 +132,18 @@ defmodule Livebook.FileSystem.S3 do defp put_id(changeset) do if bucket_url = get_field(changeset, :bucket_url) do - hash = :crypto.hash(:sha256, bucket_url) - encrypted_hash = Base.url_encode64(hash, padding: false) - - put_change(changeset, :id, "s3-#{encrypted_hash}") + put_change(changeset, :id, id(bucket_url)) else changeset end end + + defp id(bucket_url) do + hash = :crypto.hash(:sha256, bucket_url) + encrypted_hash = Base.url_encode64(hash, padding: false) + + "s3-#{encrypted_hash}" + end end defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do diff --git a/lib/livebook/hubs.ex b/lib/livebook/hubs.ex index 1bf2d7fff2a..18c83d3ffdf 100644 --- a/lib/livebook/hubs.ex +++ b/lib/livebook/hubs.ex @@ -161,9 +161,9 @@ defmodule Livebook.Hubs do Topic `hubs:file_systems`: - * `{:file_system_created, t:FileSystem}` - * `{:file_system_updated, t:FileSystem}` - * `{:file_system_deleted, t:FileSystem}` + * `{:file_system_created, FileSystem.t()}` + * `{:file_system_updated, FileSystem.t()}` + * `{:file_system_deleted, FileSystem.t()}` """ @spec subscribe(atom() | list(atom())) :: :ok | {:error, term()} @@ -316,12 +316,14 @@ defmodule Livebook.Hubs do @doc """ Gets a list of file systems for given hub. """ - @spec get_file_systems(Provider.t()) :: list(FileSystem.t()) - def get_file_systems(hub) do + @spec get_file_systems(Provider.t(), keyword()) :: list(FileSystem.t()) + def get_file_systems(hub, opts \\ []) do hub_file_systems = Provider.get_file_systems(hub) - local_file_system = Livebook.Config.local_file_system() + sorted_hub_file_systems = Enum.sort_by(hub_file_systems, & &1.id) - [local_file_system | Enum.sort_by(hub_file_systems, & &1.id)] + if opts[:hub_only], + do: sorted_hub_file_systems, + else: [Livebook.Config.local_file_system() | sorted_hub_file_systems] end @doc """ diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 625c1ef80db..951f3d735fa 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -2661,15 +2661,20 @@ defmodule Livebook.Session do {:error, "no file exists at path #{inspect(file.path)}"} end else - type = Livebook.FileSystems.type(file.file_system) - dumped_data = FileSystem.dump(file.file_system) - "/" <> key = file.path - spec = - Map.merge(dumped_data, %{ - type: String.to_existing_atom(type), - key: key - }) + case file.file_system do + %FileSystem.S3{} = file_system -> + "/" <> key = file.path + + %{ + type: :s3, + bucket_url: file_system.bucket_url, + region: file_system.region, + access_key_id: file_system.access_key_id, + secret_access_key: file_system.secret_access_key, + key: key + } + end {:ok, spec} end diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index 9b39a4beb72..2bec1f43241 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -71,12 +71,12 @@ defmodule LivebookWeb.FileSelectComponent do |> assign(assigns) |> update_file_infos(force_reload? or running_files_changed?) - configure_path = ~p"/hub/#{Livebook.Hubs.Personal.id()}/file-systems/new" - - {file_systems, configure_path} = + {file_systems, configure_hub_id} = if hub = socket.assigns[:hub], - do: {Livebook.Hubs.get_file_systems(hub), ~p"/hub/#{hub.id}/file-systems/new"}, - else: {Livebook.Hubs.get_file_systems(), configure_path} + do: {Livebook.Hubs.get_file_systems(hub), hub.id}, + else: {Livebook.Hubs.get_file_systems(), Livebook.Hubs.Personal.id()} + + configure_path = ~p"/hub/#{configure_hub_id}/file-systems/new" {:ok, assign(socket, file_systems: file_systems, configure_path: configure_path)} end diff --git a/lib/livebook_web/live/hub/edit/personal_component.ex b/lib/livebook_web/live/hub/edit/personal_component.ex index 5068b11aac5..0800c0fc7a4 100644 --- a/lib/livebook_web/live/hub/edit/personal_component.ex +++ b/lib/livebook_web/live/hub/edit/personal_component.ex @@ -11,7 +11,7 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do socket = assign(socket, assigns) changeset = Personal.change_hub(assigns.hub) secrets = Hubs.get_secrets(assigns.hub) - [_local | file_systems] = Hubs.get_file_systems(assigns.hub) + file_systems = Hubs.get_file_systems(assigns.hub, hub_only: true) secret_name = assigns.params["secret_name"] file_system_id = assigns.params["file_system_id"] @@ -101,7 +101,6 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do id="hub-secrets-list" hub={@hub} secrets={@secrets} - target={@myself} /> @@ -119,7 +118,6 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do id="hub-file-systems-list" hub_id={@hub.id} file_systems={@file_systems} - target={@myself} /> @@ -239,25 +237,6 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do {:noreply, validate(params, :stamp_changeset, socket)} end - def handle_event("delete_hub_secret", attrs, socket) do - %{hub: hub} = socket.assigns - - on_confirm = fn socket -> - {:ok, secret} = Livebook.Secrets.update_secret(%Livebook.Secrets.Secret{}, attrs) - _ = Livebook.Hubs.delete_secret(hub, secret) - - socket - end - - {:noreply, - confirm(socket, on_confirm, - title: "Delete hub secret - #{attrs["name"]}", - description: "Are you sure you want to delete this hub secret?", - confirm_text: "Delete", - confirm_icon: "delete-bin-6-line" - )} - end - defp save(params, changeset_name, socket) do case Personal.update_hub(socket.assigns.hub, params) do {:ok, hub} -> diff --git a/lib/livebook_web/live/hub/edit/team_component.ex b/lib/livebook_web/live/hub/edit/team_component.ex index c565570af81..8cbaa4796fa 100644 --- a/lib/livebook_web/live/hub/edit/team_component.ex +++ b/lib/livebook_web/live/hub/edit/team_component.ex @@ -13,7 +13,7 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do changeset = Team.change_hub(assigns.hub) show_key? = assigns.params["show-key"] == "true" secrets = Livebook.Hubs.get_secrets(assigns.hub) - [_local | file_systems] = Hubs.get_file_systems(assigns.hub) + file_systems = Hubs.get_file_systems(assigns.hub, hub_only: true) secret_name = assigns.params["secret_name"] file_system_id = assigns.params["file_system_id"] is_default? = is_default?(assigns.hub) diff --git a/lib/livebook_web/live/hub/edit_live.ex b/lib/livebook_web/live/hub/edit_live.ex index 25f59504c6b..d93861d0105 100644 --- a/lib/livebook_web/live/hub/edit_live.ex +++ b/lib/livebook_web/live/hub/edit_live.ex @@ -9,12 +9,13 @@ defmodule LivebookWeb.Hub.EditLive do @impl true def mount(_params, _session, socket) do + Hubs.subscribe([:connection]) + {:ok, assign(socket, hub: nil, type: nil, page_title: "Hub - Livebook", params: %{})} end @impl true def handle_params(params, _url, socket) do - Hubs.subscribe([:connection, :secrets, :file_systems]) hub = Hubs.fetch_hub!(params["id"]) type = Provider.type(hub) @@ -85,55 +86,8 @@ defmodule LivebookWeb.Hub.EditLive do end @impl true - def handle_info( - {:secret_created, %{name: name, hub_id: id}}, - %{assigns: %{hub: %{id: id}}} = socket - ) do - {:noreply, - socket - |> push_navigate(to: ~p"/hub/#{id}") - |> put_flash(:success, "Secret #{name} added successfully")} - end - - def handle_info( - {:secret_updated, %{name: name, hub_id: id}}, - %{assigns: %{hub: %{id: id}}} = socket - ) do - {:noreply, - socket - |> push_navigate(to: ~p"/hub/#{id}") - |> put_flash(:success, "Secret #{name} updated successfully")} - end - - def handle_info( - {:secret_deleted, %{name: name, hub_id: id}}, - %{assigns: %{hub: %{id: id}}} = socket - ) do - {:noreply, - socket - |> push_navigate(to: ~p"/hub/#{id}") - |> put_flash(:success, "Secret #{name} deleted successfully")} - end - - def handle_info({:file_system_created, _}, socket) do - {:noreply, - socket - |> push_navigate(to: ~p"/hub/#{socket.assigns.hub.id}") - |> put_flash(:success, "File storage added successfully")} - end - - def handle_info({:file_system_updated, _}, socket) do - {:noreply, - socket - |> push_navigate(to: ~p"/hub/#{socket.assigns.hub.id}") - |> put_flash(:success, "File storage updated successfully")} - end - - def handle_info({:file_system_deleted, _}, socket) do - {:noreply, - socket - |> push_navigate(to: ~p"/hub/#{socket.assigns.hub.id}") - |> put_flash(:success, "File storage deleted successfully")} + def handle_info({:redirect, id, flash}, %{assigns: %{hub: %{id: id}}} = socket) do + {:noreply, socket |> push_navigate(to: ~p"/hub/#{id}") |> put_flash(:success, flash)} end def handle_info({:hub_connected, id}, %{assigns: %{hub: %{id: id}}} = socket) do diff --git a/lib/livebook_web/live/hub/file_system_form_component.ex b/lib/livebook_web/live/hub/file_system_form_component.ex index 057bc4e79b8..4f4c359f5b6 100644 --- a/lib/livebook_web/live/hub/file_system_form_component.ex +++ b/lib/livebook_web/live/hub/file_system_form_component.ex @@ -5,7 +5,13 @@ defmodule LivebookWeb.Hub.FileSystemFormComponent do alias Livebook.FileSystems @impl true - def update(%{file_system: file_system} = assigns, socket) do + def update(assigns, socket) do + {file_system, assigns} = Map.pop!(assigns, :file_system) + + mode = mode(file_system) + button = button(file_system) + title = title(file_system) + file_system = file_system || %FileSystem.S3{} changeset = FileSystems.change_file_system(file_system) socket = assign(socket, assigns) @@ -14,9 +20,9 @@ defmodule LivebookWeb.Hub.FileSystemFormComponent do assign(socket, file_system: file_system, changeset: changeset, - mode: mode(socket), - title: title(socket), - button: button(socket), + mode: mode, + title: title, + button: button, error_message: nil )} end @@ -84,9 +90,19 @@ defmodule LivebookWeb.Hub.FileSystemFormComponent do with {:ok, file_system} <- FileSystems.update_file_system(socket.assigns.file_system, attrs), :ok <- check_file_system_conectivity(file_system), :ok <- save_file_system(file_system, socket) do - {:noreply, push_patch(socket, to: socket.assigns.return_to)} + message = + case socket.assigns.mode do + :new -> "File storage added successfully" + :edit -> "File storage updated successfully" + end + + {:noreply, + socket + |> put_flash(:success, message) + |> push_redirect(to: socket.assigns.return_to)} else {:error, %Ecto.Changeset{} = changeset} -> {:noreply, assign(socket, changeset: changeset)} + {:transport_error, message} -> {:noreply, put_flash(socket, :error, message)} {:error, message} -> {:noreply, assign(socket, error_message: message)} end end @@ -107,12 +123,12 @@ defmodule LivebookWeb.Hub.FileSystemFormComponent do end end - defp mode(%{assigns: %{file_system: nil}}), do: :new + defp mode(nil), do: :new defp mode(_), do: :edit - defp title(%{assigns: %{file_system: nil}}), do: "Add file storage" + defp title(nil), do: "Add file storage" defp title(_), do: "Edit file storage" - defp button(%{assigns: %{file_system: nil}}), do: %{icon: "add-line", label: "Add"} + defp button(nil), do: %{icon: "add-line", label: "Add"} defp button(_), do: %{icon: "save-line", label: "Save"} end diff --git a/lib/livebook_web/live/hub/file_system_list_component.ex b/lib/livebook_web/live/hub/file_system_list_component.ex index fb669ccfa14..ae6501843e1 100644 --- a/lib/livebook_web/live/hub/file_system_list_component.ex +++ b/lib/livebook_web/live/hub/file_system_list_component.ex @@ -80,8 +80,12 @@ defmodule LivebookWeb.Hub.FileSystemListComponent do file_system = Enum.find(file_systems, &(&1.id == id)) case Livebook.Hubs.delete_file_system(hub, file_system) do - :ok -> socket - {:transport_error, reason} -> put_flash(socket, :error, reason) + :ok -> + send(self(), {:redirect, hub.id, "File storage deleted successfully"}) + socket + + {:transport_error, reason} -> + put_flash(socket, :error, reason) end end diff --git a/lib/livebook_web/live/hub/secret_list_component.ex b/lib/livebook_web/live/hub/secret_list_component.ex index 015e98d92d3..1ac71c9d6d5 100644 --- a/lib/livebook_web/live/hub/secret_list_component.ex +++ b/lib/livebook_web/live/hub/secret_list_component.ex @@ -64,7 +64,7 @@ defmodule LivebookWeb.Hub.SecretListComponent do } ) } - phx-target={@target} + phx-target={@myself} role="menuitem" > <.remix_icon icon="delete-bin-line" /> @@ -92,8 +92,12 @@ defmodule LivebookWeb.Hub.SecretListComponent do hub = Hubs.fetch_hub!(secret.hub_id) case Hubs.delete_secret(hub, secret) do - :ok -> socket - {:transport_error, reason} -> put_flash(socket, :error, reason) + :ok -> + send(self(), {:redirect, hub.id, "Secret #{secret.name} deleted successfully"}) + socket + + {:transport_error, reason} -> + put_flash(socket, :error, reason) end end diff --git a/lib/livebook_web/live/session_live/persistence_component.ex b/lib/livebook_web/live/session_live/persistence_component.ex index 6a3a09f548b..6a3df30e704 100644 --- a/lib/livebook_web/live/session_live/persistence_component.ex +++ b/lib/livebook_web/live/session_live/persistence_component.ex @@ -49,7 +49,6 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do socket = socket |> assign(assigns) - |> assign_new(:hub, fn -> nil end) |> assign_new(:attrs, fn -> attrs end) |> assign_new(:new_attrs, fn -> attrs end) |> assign_new(:draft_file, fn -> diff --git a/test/livebook_web/live/hub/edit_live_test.exs b/test/livebook_web/live/hub/edit_live_test.exs index e95463691e5..128ed70f417 100644 --- a/test/livebook_web/live/hub/edit_live_test.exs +++ b/test/livebook_web/live/hub/edit_live_test.exs @@ -170,15 +170,7 @@ defmodule LivebookWeb.Hub.EditLiveTest do attrs = %{file_system: Livebook.FileSystem.dump(file_system)} - Bypass.expect_once(bypass, "GET", "/", fn conn -> - conn - |> Plug.Conn.put_resp_content_type("application/xml") - |> Plug.Conn.resp(200, """ - - mybucket - - """) - end) + expect_s3_listing(bypass) refute render(view) =~ file_system.bucket_url @@ -221,15 +213,7 @@ defmodule LivebookWeb.Hub.EditLiveTest do attrs = %{file_system: Livebook.FileSystem.dump(file_system)} - Bypass.expect_once(bypass, "GET", "/", fn conn -> - conn - |> Plug.Conn.put_resp_content_type("application/xml") - |> Plug.Conn.resp(200, """ - - mybucket - - """) - end) + expect_s3_listing(bypass) view |> element("#hub-file-system-#{file_system.id}-edit") @@ -290,4 +274,16 @@ defmodule LivebookWeb.Hub.EditLiveTest do refute file_system in Livebook.Hubs.get_file_systems(hub) end end + + defp expect_s3_listing(bypass) do + Bypass.expect_once(bypass, "GET", "/", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + end end From bb612c7d61f019f0cf6ac8f527e188cb456ecf92 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 22 Sep 2023 15:44:31 -0300 Subject: [PATCH 30/35] Apply review comments --- lib/livebook/file_system/s3.ex | 40 +++++++++++-------- lib/livebook/hubs/team_client.ex | 2 +- test/livebook_teams/hubs/team_client_test.exs | 9 +++-- test/support/factory.ex | 3 +- test/support/hub_helpers.ex | 5 +-- 5 files changed, 34 insertions(+), 25 deletions(-) diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index ae86267172d..1728ddf8f4b 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -11,7 +11,8 @@ defmodule Livebook.FileSystem.S3 do external_id: String.t() | nil, region: String.t(), access_key_id: String.t(), - secret_access_key: String.t() + secret_access_key: String.t(), + hub_id: String.t() | nil } embedded_schema do @@ -20,6 +21,7 @@ defmodule Livebook.FileSystem.S3 do field :region, :string field :access_key_id, :string field :secret_access_key, :string + field :hub_id, :string, virtual: true end @doc """ @@ -33,28 +35,24 @@ defmodule Livebook.FileSystem.S3 do * `:external_id` - the external id from Teams. - * `:prefix` - the id prefix. + * `:hub_id` - the hub id. """ @spec new(String.t(), String.t(), String.t(), keyword()) :: t() def new(bucket_url, access_key_id, secret_access_key, opts \\ []) do - opts = Keyword.validate!(opts, [:region, :external_id, :prefix]) + opts = Keyword.validate!(opts, [:region, :external_id, :hub_id]) bucket_url = String.trim_trailing(bucket_url, "/") region = opts[:region] || region_from_uri(bucket_url) - id = - if prefix = opts[:prefix], - do: "#{prefix}-#{id(bucket_url)}", - else: id(bucket_url) - %__MODULE__{ - id: id, + id: id(opts[:hub_id], bucket_url), bucket_url: bucket_url, external_id: opts[:external_id], region: region, access_key_id: access_key_id, - secret_access_key: secret_access_key + secret_access_key: secret_access_key, + hub_id: opts[:hub_id] } end @@ -115,7 +113,8 @@ defmodule Livebook.FileSystem.S3 do :external_id, :region, :access_key_id, - :secret_access_key + :secret_access_key, + :hub_id ]) |> put_region_from_uri() |> validate_required([:bucket_url, :access_key_id, :secret_access_key]) @@ -131,14 +130,21 @@ defmodule Livebook.FileSystem.S3 do end defp put_id(changeset) do - if bucket_url = get_field(changeset, :bucket_url) do - put_change(changeset, :id, id(bucket_url)) - else + hub_id = get_field(changeset, :hub_id) + bucket_url = get_field(changeset, :bucket_url) + + if get_field(changeset, :id) do changeset + else + put_change(changeset, :id, id(hub_id, bucket_url)) end end - defp id(bucket_url) do + defp id(nil, nil), do: nil + defp id(nil, bucket_url), do: hashed_id(bucket_url) + defp id(hub_id, bucket_url), do: "#{hub_id}-#{hashed_id(bucket_url)}" + + defp hashed_id(bucket_url) do hash = :crypto.hash(:sha256, bucket_url) encrypted_hash = Base.url_encode64(hash, padding: false) @@ -406,7 +412,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do region: fields["region"], access_key_id: fields["access_key_id"], secret_access_key: fields["secret_access_key"], - prefix: fields["prefix"] + hub_id: fields["hub_id"] }) end @@ -414,7 +420,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do S3.new(fields.bucket_url, fields.access_key_id, fields.secret_access_key, region: fields[:region], external_id: fields[:external_id], - prefix: fields[:prefix] + hub_id: fields[:hub_id] ) end diff --git a/lib/livebook/hubs/team_client.ex b/lib/livebook/hubs/team_client.ex index 526262a3e13..e430dc951f0 100644 --- a/lib/livebook/hubs/team_client.ex +++ b/lib/livebook/hubs/team_client.ex @@ -190,7 +190,7 @@ defmodule Livebook.Hubs.TeamClient do dumped_data = Map.merge(Jason.decode!(decrypted_value), %{ "external_id" => file_system.id, - "prefix" => state.hub.id + "hub_id" => state.hub.id }) FileSystems.load(file_system.type, dumped_data) diff --git a/test/livebook_teams/hubs/team_client_test.exs b/test/livebook_teams/hubs/team_client_test.exs index 6f446455822..90f8c29d48e 100644 --- a/test/livebook_teams/hubs/team_client_test.exs +++ b/test/livebook_teams/hubs/team_client_test.exs @@ -311,7 +311,8 @@ defmodule Livebook.Hubs.TeamClientTest do hash = :crypto.hash(:sha256, bucket_url) fs_id = "#{id}-s3-#{Base.url_encode64(hash, padding: false)}" - file_system = build(:fs_s3, id: fs_id, bucket_url: bucket_url, external_id: "123456") + file_system = + build(:fs_s3, id: fs_id, bucket_url: bucket_url, external_id: "123456", hub_id: id) type = Livebook.FileSystems.type(file_system) %{name: name} = Livebook.FileSystem.external_metadata(file_system) @@ -353,7 +354,8 @@ defmodule Livebook.Hubs.TeamClientTest do hash = :crypto.hash(:sha256, bucket_url) fs_id = "#{id}-s3-#{Base.url_encode64(hash, padding: false)}" - file_system = build(:fs_s3, id: fs_id, bucket_url: bucket_url, external_id: "994641") + file_system = + build(:fs_s3, id: fs_id, bucket_url: bucket_url, external_id: "994641", hub_id: id) type = Livebook.FileSystems.type(file_system) %{name: name} = Livebook.FileSystem.external_metadata(file_system) @@ -425,7 +427,8 @@ defmodule Livebook.Hubs.TeamClientTest do hash = :crypto.hash(:sha256, bucket_url) fs_id = "#{id}-s3-#{Base.url_encode64(hash, padding: false)}" - file_system = build(:fs_s3, id: fs_id, bucket_url: bucket_url, external_id: "45465641") + file_system = + build(:fs_s3, id: fs_id, bucket_url: bucket_url, external_id: "45465641", hub_id: id) type = Livebook.FileSystems.type(file_system) %{name: name} = Livebook.FileSystem.external_metadata(file_system) diff --git a/test/support/factory.ex b/test/support/factory.ex index 144e798ad6a..096d21c0623 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -77,7 +77,8 @@ defmodule Livebook.Factory do external_id: nil, region: "us-east-1", access_key_id: "key", - secret_access_key: "secret" + secret_access_key: "secret", + hub_id: nil } end diff --git a/test/support/hub_helpers.ex b/test/support/hub_helpers.ex index c84e4a66c79..5ec99e7c215 100644 --- a/test/support/hub_helpers.ex +++ b/test/support/hub_helpers.ex @@ -103,9 +103,8 @@ defmodule Livebook.HubHelpers do end def create_teams_file_system(hub, node) do - org_key = :erpc.call(node, Hub.Integration, :get_org_key!, [hub.org_key_id]) - - :erpc.call(node, Hub.Integration, :create_file_system, [[org_key: org_key]]) + org_key = erpc_call(node, :get_org_key!, [hub.org_key_id]) + erpc_call(node, :create_file_system, [[org_key: org_key]]) end def build_bypass_file_system(bypass) do From 683ed6f77a0014971e53cb7dc4193bc2bc0a7efe Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 22 Sep 2023 15:48:26 -0300 Subject: [PATCH 31/35] Fix test --- test/livebook_teams/teams_test.exs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/livebook_teams/teams_test.exs b/test/livebook_teams/teams_test.exs index 7963239dcd2..e7262d20989 100644 --- a/test/livebook_teams/teams_test.exs +++ b/test/livebook_teams/teams_test.exs @@ -295,7 +295,12 @@ defmodule Livebook.TeamsTest do test "returns transport errors when file system doesn't exists", %{user: user, node: node} do hub = create_team_hub(user, node) - file_system = build(:fs_s3, bucket_url: "https://i_cant_exist.s3.amazonaws.com") + + file_system = + build(:fs_s3, + bucket_url: "https://i_cant_exist.s3.amazonaws.com", + external_id: "123456789" + ) # Guarantee it doesn't exists and will return HTTP status 404 assert Teams.delete_file_system(hub, file_system) == From bcc803aeea0f874d295ad813d108bc143765b5ae Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 22 Sep 2023 18:05:36 -0300 Subject: [PATCH 32/35] Improve tests --- .../livebook_teams/web/hub/edit_live_test.exs | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/test/livebook_teams/web/hub/edit_live_test.exs b/test/livebook_teams/web/hub/edit_live_test.exs index ab66f7b5c0d..e4c2be9331f 100644 --- a/test/livebook_teams/web/hub/edit_live_test.exs +++ b/test/livebook_teams/web/hub/edit_live_test.exs @@ -261,15 +261,7 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do id = "#{hub.id}-#{file_system.id}" attrs = %{file_system: Livebook.FileSystem.dump(file_system)} - Bypass.expect_once(bypass, "GET", "/", fn conn -> - conn - |> Plug.Conn.put_resp_content_type("application/xml") - |> Plug.Conn.resp(200, """ - - mybucket - - """) - end) + expect_s3_listing(bypass) refute render(view) =~ file_system.bucket_url @@ -315,15 +307,7 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do attrs = %{file_system: Livebook.FileSystem.dump(file_system)} - Bypass.expect_once(bypass, "GET", "/", fn conn -> - conn - |> Plug.Conn.put_resp_content_type("application/xml") - |> Plug.Conn.resp(200, """ - - mybucket - - """) - end) + expect_s3_listing(bypass) view |> element("#hub-file-system-#{file_system.id}-edit") @@ -387,4 +371,16 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do refute file_system in Livebook.Hubs.get_file_systems(hub) end end + + defp expect_s3_listing(bypass) do + Bypass.expect_once(bypass, "GET", "/", fn conn -> + conn + |> Plug.Conn.put_resp_content_type("application/xml") + |> Plug.Conn.resp(200, """ + + mybucket + + """) + end) + end end From ed9476e5f021ba9a469e71cb3577fa2b6aecf9ec Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 22 Sep 2023 18:06:53 -0300 Subject: [PATCH 33/35] Apply review comments --- lib/livebook/file_system/s3.ex | 23 ++++++++++++------- lib/livebook/hubs/team_client.ex | 7 +++--- test/livebook/file_system/s3_test.exs | 4 +++- .../livebook_teams/web/hub/edit_live_test.exs | 12 +++++----- test/livebook_teams/web/session_live_test.exs | 17 ++++++++++---- test/support/hub_helpers.ex | 8 +++---- 6 files changed, 44 insertions(+), 27 deletions(-) diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index 1728ddf8f4b..f7ab311712a 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -35,24 +35,29 @@ defmodule Livebook.FileSystem.S3 do * `:external_id` - the external id from Teams. - * `:hub_id` - the hub id. + * `:hub_id` - the Hub id. + + * `:id` - the file system id. """ @spec new(String.t(), String.t(), String.t(), keyword()) :: t() def new(bucket_url, access_key_id, secret_access_key, opts \\ []) do - opts = Keyword.validate!(opts, [:region, :external_id, :hub_id]) + opts = Keyword.validate!(opts, [:region, :external_id, :hub_id, :id]) bucket_url = String.trim_trailing(bucket_url, "/") region = opts[:region] || region_from_uri(bucket_url) + hub_id = opts[:hub_id] + id = opts[:id] || id(hub_id, bucket_url) + %__MODULE__{ - id: id(opts[:hub_id], bucket_url), + id: id, bucket_url: bucket_url, external_id: opts[:external_id], region: region, access_key_id: access_key_id, secret_access_key: secret_access_key, - hub_id: opts[:hub_id] + hub_id: hub_id } end @@ -140,9 +145,9 @@ defmodule Livebook.FileSystem.S3 do end end - defp id(nil, nil), do: nil - defp id(nil, bucket_url), do: hashed_id(bucket_url) - defp id(hub_id, bucket_url), do: "#{hub_id}-#{hashed_id(bucket_url)}" + def id(nil, nil), do: nil + def id(nil, bucket_url), do: hashed_id(bucket_url) + def id(hub_id, bucket_url), do: "#{hub_id}-#{hashed_id(bucket_url)}" defp hashed_id(bucket_url) do hash = :crypto.hash(:sha256, bucket_url) @@ -412,6 +417,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do region: fields["region"], access_key_id: fields["access_key_id"], secret_access_key: fields["secret_access_key"], + id: fields["id"], hub_id: fields["hub_id"] }) end @@ -420,6 +426,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do S3.new(fields.bucket_url, fields.access_key_id, fields.secret_access_key, region: fields[:region], external_id: fields[:external_id], + id: fields[:id], hub_id: fields[:hub_id] ) end @@ -427,7 +434,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do def dump(file_system) do file_system |> Map.from_struct() - |> Map.take([:bucket_url, :region, :access_key_id, :secret_access_key]) + |> Map.take([:id, :bucket_url, :region, :access_key_id, :secret_access_key, :hub_id]) end def external_metadata(file_system) do diff --git a/lib/livebook/hubs/team_client.ex b/lib/livebook/hubs/team_client.ex index e430dc951f0..e829a05f7a2 100644 --- a/lib/livebook/hubs/team_client.ex +++ b/lib/livebook/hubs/team_client.ex @@ -188,10 +188,9 @@ defmodule Livebook.Hubs.TeamClient do {:ok, decrypted_value} = Teams.decrypt(file_system.value, secret_key, sign_secret) dumped_data = - Map.merge(Jason.decode!(decrypted_value), %{ - "external_id" => file_system.id, - "hub_id" => state.hub.id - }) + decrypted_value + |> Jason.decode!() + |> Map.put("external_id", file_system.id) FileSystems.load(file_system.type, dumped_data) end diff --git a/test/livebook/file_system/s3_test.exs b/test/livebook/file_system/s3_test.exs index fc96a3b66fc..0a3cbf16148 100644 --- a/test/livebook/file_system/s3_test.exs +++ b/test/livebook/file_system/s3_test.exs @@ -1057,10 +1057,12 @@ defmodule Livebook.FileSystem.S3Test do file_system = build(:fs_s3) assert FileSystem.dump(file_system) == %{ + id: "s3-86IzUeRugmgK2-X2FmlkurFD0UPsr4Qs1IwieDqfQpA", bucket_url: "https://mybucket.s3.amazonaws.com", region: "us-east-1", access_key_id: "key", - secret_access_key: "secret" + secret_access_key: "secret", + hub_id: nil } end end diff --git a/test/livebook_teams/web/hub/edit_live_test.exs b/test/livebook_teams/web/hub/edit_live_test.exs index e4c2be9331f..559eea4a70f 100644 --- a/test/livebook_teams/web/hub/edit_live_test.exs +++ b/test/livebook_teams/web/hub/edit_live_test.exs @@ -257,8 +257,8 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}") bypass = Bypass.open() - file_system = build_bypass_file_system(bypass) - id = "#{hub.id}-#{file_system.id}" + file_system = build_bypass_file_system(bypass, hub.id) + id = file_system.id attrs = %{file_system: Livebook.FileSystem.dump(file_system)} expect_s3_listing(bypass) @@ -297,8 +297,8 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do test "updates existing file system", %{conn: conn, hub: hub} do bypass = Bypass.open() - file_system = build_bypass_file_system(bypass) - id = "#{hub.id}-#{file_system.id}" + file_system = build_bypass_file_system(bypass, hub.id) + id = file_system.id :ok = Hubs.create_file_system(hub, file_system) assert_receive {:file_system_created, %{id: ^id} = file_system} @@ -342,8 +342,8 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do test "detaches existing file system", %{conn: conn, hub: hub} do bypass = Bypass.open() - file_system = build_bypass_file_system(bypass) - id = "#{hub.id}-#{file_system.id}" + file_system = build_bypass_file_system(bypass, hub.id) + id = file_system.id :ok = Hubs.create_file_system(hub, file_system) assert_receive {:file_system_created, %{id: ^id} = file_system} diff --git a/test/livebook_teams/web/session_live_test.exs b/test/livebook_teams/web/session_live_test.exs index 7b3ba35d341..fd9199bece5 100644 --- a/test/livebook_teams/web/session_live_test.exs +++ b/test/livebook_teams/web/session_live_test.exs @@ -286,12 +286,21 @@ defmodule LivebookWeb.Integration.SessionLiveTest do Livebook.Hubs.subscribe([:file_systems]) personal_id = Livebook.Hubs.Personal.id() - file_system = build(:fs_s3) - Livebook.Hubs.Personal.save_file_system(file_system) + personal_file_system = build(:fs_s3) + Livebook.Hubs.Personal.save_file_system(personal_file_system) team = create_team_hub(user, node) team_id = team.id + bucket_url = "https://my-own-bucket.s3.amazonaws.com" + + file_system = + build(:fs_s3, + id: Livebook.FileSystem.S3.id(team_id, bucket_url), + bucket_url: bucket_url, + hub_id: team_id + ) + Livebook.Hubs.create_file_system(team, file_system) assert_receive {:file_system_created, team_file_system} @@ -308,7 +317,7 @@ defmodule LivebookWeb.Integration.SessionLiveTest do # checks the file systems from Personal assert has_element?(file_system_menu, "#file-system-local") - assert has_element?(file_system_menu, "#file-system-#{file_system.id}") + assert has_element?(file_system_menu, "#file-system-#{personal_file_system.id}") refute has_element?(file_system_menu, "#file-system-#{team_file_system.id}") # change the hub to Team @@ -320,7 +329,7 @@ defmodule LivebookWeb.Integration.SessionLiveTest do file_system_menu = with_target(view, "#file-system-menu") assert has_element?(file_system_menu, "#file-system-local") - refute has_element?(file_system_menu, "#file-system-#{file_system.id}") + refute has_element?(file_system_menu, "#file-system-#{personal_file_system.id}") assert has_element?(file_system_menu, "#file-system-#{team_file_system.id}") end end diff --git a/test/support/hub_helpers.ex b/test/support/hub_helpers.ex index 5ec99e7c215..6e20509e1ec 100644 --- a/test/support/hub_helpers.ex +++ b/test/support/hub_helpers.ex @@ -107,15 +107,15 @@ defmodule Livebook.HubHelpers do erpc_call(node, :create_file_system, [[org_key: org_key]]) end - def build_bypass_file_system(bypass) do + def build_bypass_file_system(bypass, hub_id \\ nil) do bucket_url = "http://localhost:#{bypass.port}" - hash = :crypto.hash(:sha256, bucket_url) file_system = build(:fs_s3, - id: "s3-#{Base.url_encode64(hash, padding: false)}", + id: Livebook.FileSystem.S3.id(hub_id, bucket_url), bucket_url: bucket_url, - region: "auto" + region: "auto", + hub_id: hub_id ) file_system From 85e9e96b5de248d2e8e6c5add01996f72ab5639f Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Fri, 22 Sep 2023 18:20:03 -0300 Subject: [PATCH 34/35] Make `personal-hub` as default `:hub_id` --- lib/livebook/file_system/s3.ex | 4 ++-- test/livebook/file_system/s3_test.exs | 4 ++-- test/support/factory.ex | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index f7ab311712a..514042babff 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -12,7 +12,7 @@ defmodule Livebook.FileSystem.S3 do region: String.t(), access_key_id: String.t(), secret_access_key: String.t(), - hub_id: String.t() | nil + hub_id: String.t() } embedded_schema do @@ -47,7 +47,7 @@ defmodule Livebook.FileSystem.S3 do bucket_url = String.trim_trailing(bucket_url, "/") region = opts[:region] || region_from_uri(bucket_url) - hub_id = opts[:hub_id] + hub_id = Keyword.get(opts, :hub_id, Livebook.Hubs.Personal.id()) id = opts[:id] || id(hub_id, bucket_url) %__MODULE__{ diff --git a/test/livebook/file_system/s3_test.exs b/test/livebook/file_system/s3_test.exs index 0a3cbf16148..c7bdd1ca0e5 100644 --- a/test/livebook/file_system/s3_test.exs +++ b/test/livebook/file_system/s3_test.exs @@ -1057,12 +1057,12 @@ defmodule Livebook.FileSystem.S3Test do file_system = build(:fs_s3) assert FileSystem.dump(file_system) == %{ - id: "s3-86IzUeRugmgK2-X2FmlkurFD0UPsr4Qs1IwieDqfQpA", + id: "personal-hub-s3-86IzUeRugmgK2-X2FmlkurFD0UPsr4Qs1IwieDqfQpA", bucket_url: "https://mybucket.s3.amazonaws.com", region: "us-east-1", access_key_id: "key", secret_access_key: "secret", - hub_id: nil + hub_id: "personal-hub" } end end diff --git a/test/support/factory.ex b/test/support/factory.ex index 096d21c0623..76a8337d86d 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -70,15 +70,16 @@ defmodule Livebook.Factory do def build(:fs_s3) do bucket_url = "https://mybucket.s3.amazonaws.com" hash = :crypto.hash(:sha256, bucket_url) + hub_id = Livebook.Hubs.Personal.id() %Livebook.FileSystem.S3{ - id: "s3-#{Base.url_encode64(hash, padding: false)}", + id: "#{hub_id}-s3-#{Base.url_encode64(hash, padding: false)}", bucket_url: bucket_url, external_id: nil, region: "us-east-1", access_key_id: "key", secret_access_key: "secret", - hub_id: nil + hub_id: hub_id } end From f1ae2200fc8158a2f7b8920e4dd8b3087271ba79 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Mon, 25 Sep 2023 10:50:14 -0300 Subject: [PATCH 35/35] Apply review comments --- lib/livebook/file_system/s3.ex | 4 +- .../live/hub/edit/personal_component.ex | 2 +- .../live/hub/edit/team_component.ex | 2 +- lib/livebook_web/live/hub/edit_live.ex | 4 - .../live/hub/file_system_list_component.ex | 3 +- .../live/hub/secret_list_component.ex | 3 +- .../add_file_system_component.ex | 113 ------------------ .../settings_live/file_systems_component.ex | 68 ----------- 8 files changed, 8 insertions(+), 191 deletions(-) delete mode 100644 lib/livebook_web/live/settings_live/add_file_system_component.ex delete mode 100644 lib/livebook_web/live/settings_live/file_systems_component.ex diff --git a/lib/livebook/file_system/s3.ex b/lib/livebook/file_system/s3.ex index 514042babff..c184cefb6c7 100644 --- a/lib/livebook/file_system/s3.ex +++ b/lib/livebook/file_system/s3.ex @@ -21,7 +21,7 @@ defmodule Livebook.FileSystem.S3 do field :region, :string field :access_key_id, :string field :secret_access_key, :string - field :hub_id, :string, virtual: true + field :hub_id, :string end @doc """ @@ -145,7 +145,7 @@ defmodule Livebook.FileSystem.S3 do end end - def id(nil, nil), do: nil + def id(_, nil), do: nil def id(nil, bucket_url), do: hashed_id(bucket_url) def id(hub_id, bucket_url), do: "#{hub_id}-#{hashed_id(bucket_url)}" diff --git a/lib/livebook_web/live/hub/edit/personal_component.ex b/lib/livebook_web/live/hub/edit/personal_component.ex index 0800c0fc7a4..53ca668b0a9 100644 --- a/lib/livebook_web/live/hub/edit/personal_component.ex +++ b/lib/livebook_web/live/hub/edit/personal_component.ex @@ -110,7 +110,7 @@ defmodule LivebookWeb.Hub.Edit.PersonalComponent do

- File storages are used to store notebooks. + File storages are used to store notebooks and their files.

<.live_component diff --git a/lib/livebook_web/live/hub/edit/team_component.ex b/lib/livebook_web/live/hub/edit/team_component.ex index 8cbaa4796fa..3bee222749d 100644 --- a/lib/livebook_web/live/hub/edit/team_component.ex +++ b/lib/livebook_web/live/hub/edit/team_component.ex @@ -180,7 +180,7 @@ defmodule LivebookWeb.Hub.Edit.TeamComponent do

- File storages are used to store notebooks. + File storages are used to store notebooks and their files.

<.live_component diff --git a/lib/livebook_web/live/hub/edit_live.ex b/lib/livebook_web/live/hub/edit_live.ex index d93861d0105..caaaa1f175d 100644 --- a/lib/livebook_web/live/hub/edit_live.ex +++ b/lib/livebook_web/live/hub/edit_live.ex @@ -86,10 +86,6 @@ defmodule LivebookWeb.Hub.EditLive do end @impl true - def handle_info({:redirect, id, flash}, %{assigns: %{hub: %{id: id}}} = socket) do - {:noreply, socket |> push_navigate(to: ~p"/hub/#{id}") |> put_flash(:success, flash)} - end - def handle_info({:hub_connected, id}, %{assigns: %{hub: %{id: id}}} = socket) do {:noreply, push_navigate(socket, to: ~p"/hub/#{id}")} end diff --git a/lib/livebook_web/live/hub/file_system_list_component.ex b/lib/livebook_web/live/hub/file_system_list_component.ex index ae6501843e1..1208a184307 100644 --- a/lib/livebook_web/live/hub/file_system_list_component.ex +++ b/lib/livebook_web/live/hub/file_system_list_component.ex @@ -81,8 +81,9 @@ defmodule LivebookWeb.Hub.FileSystemListComponent do case Livebook.Hubs.delete_file_system(hub, file_system) do :ok -> - send(self(), {:redirect, hub.id, "File storage deleted successfully"}) socket + |> put_flash(:success, "File storage deleted successfully") + |> push_navigate(to: ~p"/hub/#{hub.id}") {:transport_error, reason} -> put_flash(socket, :error, reason) diff --git a/lib/livebook_web/live/hub/secret_list_component.ex b/lib/livebook_web/live/hub/secret_list_component.ex index 1ac71c9d6d5..519e55a6a63 100644 --- a/lib/livebook_web/live/hub/secret_list_component.ex +++ b/lib/livebook_web/live/hub/secret_list_component.ex @@ -93,8 +93,9 @@ defmodule LivebookWeb.Hub.SecretListComponent do case Hubs.delete_secret(hub, secret) do :ok -> - send(self(), {:redirect, hub.id, "Secret #{secret.name} deleted successfully"}) socket + |> put_flash(:success, "Secret #{secret.name} deleted successfully") + |> push_navigate(to: ~p"/hub/#{hub.id}") {:transport_error, reason} -> put_flash(socket, :error, reason) diff --git a/lib/livebook_web/live/settings_live/add_file_system_component.ex b/lib/livebook_web/live/settings_live/add_file_system_component.ex deleted file mode 100644 index 0c00b35fa78..00000000000 --- a/lib/livebook_web/live/settings_live/add_file_system_component.ex +++ /dev/null @@ -1,113 +0,0 @@ -defmodule LivebookWeb.SettingsLive.AddFileSystemComponent do - use LivebookWeb, :live_component - - import Ecto.Changeset - - alias Livebook.FileSystem - - @impl true - def mount(socket) do - {:ok, assign(socket, changeset: changeset(), error_message: nil)} - end - - defp changeset(attrs \\ %{}) do - data = %{bucket_url: nil, region: nil, access_key_id: nil, secret_access_key: nil} - - types = %{ - bucket_url: :string, - region: :string, - access_key_id: :string, - secret_access_key: :string - } - - cast({data, types}, attrs, [:bucket_url, :region, :access_key_id, :secret_access_key]) - |> validate_required([:bucket_url, :access_key_id, :secret_access_key]) - |> Livebook.Utils.validate_url(:bucket_url) - end - - @impl true - def render(assigns) do - ~H""" -
-

- Add file system -

-

- Configure an AWS S3 bucket as a Livebook file system. - Many storage services offer an S3-compatible API and - those work as well. -

-
- <%= @error_message %> -
- <.form - :let={f} - for={@changeset} - as={:data} - phx-target={@myself} - phx-submit="add" - phx-change="validate" - autocomplete="off" - spellcheck="false" - > -
- <.text_field - field={f[:bucket_url]} - label="Bucket URL" - placeholder="https://s3.[region].amazonaws.com/[bucket]" - /> - <.text_field field={f[:region]} label="Region (optional)" /> - <.password_field field={f[:access_key_id]} label="Access Key ID" /> - <.password_field field={f[:secret_access_key]} label="Secret Access Key" /> -
- - <.link patch={@return_to} class="button-base button-outlined-gray"> - Cancel - -
-
- -
- """ - end - - @impl true - def handle_event("validate", %{"data" => data}, socket) do - changeset = data |> changeset() |> Map.replace!(:action, :validate) - {:noreply, assign(socket, changeset: changeset)} - end - - def handle_event("add", %{"data" => data}, socket) do - data - |> changeset() - |> apply_action(:insert) - |> case do - {:ok, data} -> - file_system = - FileSystem.S3.new(data.bucket_url, data.access_key_id, data.secret_access_key, - region: data.region - ) - - default_dir = FileSystem.File.new(file_system) - - case FileSystem.File.list(default_dir) do - {:ok, _} -> - Livebook.Settings.save_file_system(file_system) - send(self(), {:file_systems_updated, Livebook.Settings.file_systems()}) - {:noreply, push_patch(socket, to: socket.assigns.return_to)} - - {:error, message} -> - {:noreply, - assign(socket, - changeset: changeset(data), - error_message: "Connection test failed: " <> message - )} - end - - {:error, changeset} -> - {:noreply, assign(socket, changeset: changeset)} - end - end -end diff --git a/lib/livebook_web/live/settings_live/file_systems_component.ex b/lib/livebook_web/live/settings_live/file_systems_component.ex deleted file mode 100644 index 0a12a2f867d..00000000000 --- a/lib/livebook_web/live/settings_live/file_systems_component.ex +++ /dev/null @@ -1,68 +0,0 @@ -defmodule LivebookWeb.SettingsLive.FileSystemsComponent do - use LivebookWeb, :live_component - - alias Livebook.FileSystem - - @impl true - def render(assigns) do - ~H""" -
-
-
-
- <.file_system_info file_system={file_system} /> -
- <.file_system_actions file_system_id={file_system.id} /> -
-
-
- <.link patch={~p"/settings/add-file-system"} class="button-base button-blue"> - Add file system - -
-
- """ - end - - defp file_system_info(%{file_system: %FileSystem.Local{}} = assigns) do - ~H""" - <.labeled_text label="Type">Local disk - """ - end - - defp file_system_info(%{file_system: %FileSystem.S3{}} = assigns) do - ~H""" - <.labeled_text label="Type">S3 - <.labeled_text label="Bucket URL"><%= @file_system.bucket_url %> - """ - end - - defp file_system_actions(assigns) do - ~H""" -
- <.menu :if={@file_system_id != "local"} id={"file-system-#{@file_system_id}-menu"}> - <:toggle> - - - <.menu_item variant={:danger}> - - - -
- """ - end -end