From 72d18e5a383010a66c9ac2ceb43a4c0ebd0825ff Mon Sep 17 00:00:00 2001 From: ByeongUk Choi Date: Wed, 12 Jul 2023 20:28:51 +0900 Subject: [PATCH] Add default directory setting (#2046) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jonatan Kłosko --- lib/livebook/file_system.ex | 7 ++ lib/livebook/migration.ex | 26 +++++-- lib/livebook/settings.ex | 71 +++++++++---------- .../live/file_select_component.ex | 12 ++++ lib/livebook_web/live/open_live.ex | 2 +- lib/livebook_web/live/settings_live.ex | 20 +----- .../settings_live/file_systems_component.ex | 28 +------- 7 files changed, 81 insertions(+), 85 deletions(-) diff --git a/lib/livebook/file_system.ex b/lib/livebook/file_system.ex index 64d648b66..e61cc4f78 100644 --- a/lib/livebook/file_system.ex +++ b/lib/livebook/file_system.ex @@ -4,6 +4,13 @@ defprotocol Livebook.FileSystem do # This protocol defines an interface for file systems # that can be plugged into Livebook. + @typedoc """ + An identifier uniquely identifying the given file system. + + Every file system struct is expected have an `:id` field. + """ + @type id :: String.t() + @typedoc """ A path uniquely idenfies file in the file system. diff --git a/lib/livebook/migration.ex b/lib/livebook/migration.ex index b4576f2f6..a09716ece 100644 --- a/lib/livebook/migration.ex +++ b/lib/livebook/migration.ex @@ -11,6 +11,7 @@ defmodule Livebook.Migration do move_app_secrets_to_personal_hub() add_personal_hub_secret_key() update_file_systems_to_deterministic_ids() + move_default_file_system_id_to_default_dir() end defp delete_local_host_hub() do @@ -73,11 +74,10 @@ defmodule Livebook.Migration do end # Remap default file system id - - default_file_system_id = Livebook.Settings.default_file_system_id() - - if new_id = id_mapping[default_file_system_id] do - Livebook.Settings.set_default_file_system(new_id) + with {:ok, default_file_system_id} <- + Livebook.Storage.fetch_key(:settings, "global", :default_file_system_id), + {:ok, new_id} <- Map.fetch(id_mapping, default_file_system_id) do + Livebook.Storage.insert(:settings, "global", default_file_system_id: new_id) end # Livebook.NotebookManager is started before the migration runs, @@ -87,4 +87,20 @@ defmodule Livebook.Migration do # notebooks at this point (and the user can easily star again) end end + + defp move_default_file_system_id_to_default_dir() do + # Convert default_file_system_id to default_dir + # TODO: remove on Livebook v0.12 + + with {:ok, default_file_system_id} <- + Livebook.Storage.fetch_key(:settings, "global", :default_file_system_id) do + with {:ok, default_file_system} <- + Livebook.Settings.fetch_file_system(default_file_system_id) do + default_dir = Livebook.FileSystem.File.new(default_file_system) + Livebook.Settings.set_default_dir(default_dir) + end + + Livebook.Storage.delete_key(:settings, "global", :default_file_system_id) + end + end end diff --git a/lib/livebook/settings.ex b/lib/livebook/settings.ex index c45ae3fcb..78b917605 100644 --- a/lib/livebook/settings.ex +++ b/lib/livebook/settings.ex @@ -9,11 +9,6 @@ defmodule Livebook.Settings do alias Livebook.Storage alias Livebook.Settings.EnvVar - @typedoc """ - An id that is used for file system's manipulation, either insertion or removal. - """ - @type file_system_id :: String.t() - @doc """ Returns the current autosave path. """ @@ -62,6 +57,22 @@ defmodule Livebook.Settings do [Livebook.Config.local_file_system() | restored_file_systems] end + @doc """ + Finds a file system by id. + """ + @spec fetch_file_system(FileSystem.id()) :: {:ok, FileSystem.t()} + def fetch_file_system(file_system_id) do + local_file_system = Livebook.Config.local_file_system() + + if file_system_id == local_file_system.id do + {:ok, local_file_system} + else + with {:ok, config} <- Storage.fetch(:file_systems, file_system_id) do + {:ok, storage_to_fs(config)} + end + end + end + @doc """ Saves a new file system to the configured ones. """ @@ -79,15 +90,15 @@ defmodule Livebook.Settings do @doc """ Removes the given file system from the configured ones. """ - @spec remove_file_system(file_system_id()) :: :ok + @spec remove_file_system(FileSystem.id()) :: :ok def remove_file_system(file_system_id) do - if default_file_system_id() == file_system_id do - Livebook.Storage.delete_key(:settings, "global", :default_file_system_id) + if default_dir().file_system.id == file_system_id do + Storage.delete_key(:settings, "global", :default_dir) end Livebook.NotebookManager.remove_file_system(file_system_id) - Livebook.Storage.delete(:file_systems, file_system_id) + Storage.delete(:file_systems, file_system_id) end defp storage_to_fs(%{type: "s3"} = config) do @@ -222,38 +233,26 @@ defmodule Livebook.Settings do end @doc """ - Sets default file system. + Sets default directory. """ - @spec set_default_file_system(file_system_id()) :: :ok - def set_default_file_system(file_system_id) do - Livebook.Storage.insert(:settings, "global", default_file_system_id: file_system_id) + @spec set_default_dir(FileSystem.File.t()) :: :ok + def set_default_dir(file) do + Storage.insert(:settings, "global", + default_dir: %{file_system_id: file.file_system.id, path: file.path} + ) end @doc """ - Returns the default file system. + Gets default directory. """ - @spec default_file_system() :: Filesystem.t() - def default_file_system() do - case Livebook.Storage.fetch(:file_systems, default_file_system_id()) do - {:ok, file} -> storage_to_fs(file) - :error -> Livebook.Config.local_file_system() + @spec default_dir() :: FileSystem.File.t() + def default_dir() do + with {:ok, %{file_system_id: file_system_id, path: path}} <- + Storage.fetch_key(:settings, "global", :default_dir), + {:ok, file_system} <- fetch_file_system(file_system_id) do + FileSystem.File.new(file_system, path) + else + _ -> FileSystem.File.new(Livebook.Config.local_file_system()) end end - - @doc """ - Returns the default file system id. - """ - @spec default_file_system_id() :: file_system_id() - def default_file_system_id() do - case Livebook.Storage.fetch_key(:settings, "global", :default_file_system_id) do - {:ok, default_file_system_id} -> default_file_system_id - :error -> "local" - end - end - - @doc """ - Returns the home directory in the default file system. - """ - @spec default_file_system_home() :: FileSystem.File.t() - def default_file_system_home(), do: FileSystem.File.new(default_file_system()) end diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index ab1a8d304..0f1d1a982 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -121,6 +121,12 @@ defmodule LivebookWeb.FileSelectComponent do New notebook + <.menu_item> + +
<%= render_slot(@inner_block) %> @@ -560,6 +566,12 @@ defmodule LivebookWeb.FileSelectComponent do {:noreply, assign(socket, error: false)} end + def handle_event("set_default_directory", _params, socket) do + {dir, _prefix} = dir_and_prefix(socket.assigns.file) + Livebook.Settings.set_default_dir(dir) + {:noreply, socket} + end + defp update_file_infos(%{assigns: assigns} = socket, force_reload?) do current_file_infos = assigns[:file_infos] || [] {dir, prefix} = dir_and_prefix(assigns.file) diff --git a/lib/livebook_web/live/open_live.ex b/lib/livebook_web/live/open_live.ex index 22e6e731e..d2ccc639c 100644 --- a/lib/livebook_web/live/open_live.ex +++ b/lib/livebook_web/live/open_live.ex @@ -250,7 +250,7 @@ defmodule LivebookWeb.OpenLive do end end - defp file_from_params(_params), do: Livebook.Settings.default_file_system_home() + defp file_from_params(_params), do: Livebook.Settings.default_dir() defp import_source(socket, source, session_opts) do {notebook, %{warnings: messages}} = Livebook.LiveMarkdown.notebook_from_livemd(source) diff --git a/lib/livebook_web/live/settings_live.ex b/lib/livebook_web/live/settings_live.ex index a7704987d..8def5b84d 100644 --- a/lib/livebook_web/live/settings_live.ex +++ b/lib/livebook_web/live/settings_live.ex @@ -21,8 +21,7 @@ defmodule LivebookWeb.SettingsLive do dialog_opened?: false }, update_check_enabled: Livebook.UpdateCheck.enabled?(), - page_title: "Settings - Livebook", - default_file_system_id: Livebook.Settings.default_file_system_id() + page_title: "Settings - Livebook" )} end @@ -110,10 +109,7 @@ defmodule LivebookWeb.SettingsLive do is visible only to the current machine, but alternative file systems are available, such as S3-based storages.

- +
@@ -307,13 +303,8 @@ defmodule LivebookWeb.SettingsLive do def handle_event("detach_file_system", %{"id" => file_system_id}, socket) do on_confirm = fn socket -> Livebook.Settings.remove_file_system(file_system_id) - file_systems = Livebook.Settings.file_systems() - - assign(socket, - file_systems: file_systems, - default_file_system_id: Livebook.Settings.default_file_system_id() - ) + assign(socket, file_systems: file_systems) end {:noreply, @@ -326,11 +317,6 @@ defmodule LivebookWeb.SettingsLive do )} end - def handle_event("make_default_file_system", %{"id" => file_system_id}, socket) do - Livebook.Settings.set_default_file_system(file_system_id) - {:noreply, assign(socket, default_file_system_id: file_system_id)} - end - def handle_event("save", %{"update_check_enabled" => enabled}, socket) do enabled = enabled == "true" Livebook.UpdateCheck.set_enabled(enabled) diff --git a/lib/livebook_web/live/settings_live/file_systems_component.ex b/lib/livebook_web/live/settings_live/file_systems_component.ex index 9889fa434..0a12a2f86 100644 --- a/lib/livebook_web/live/settings_live/file_systems_component.ex +++ b/lib/livebook_web/live/settings_live/file_systems_component.ex @@ -15,10 +15,7 @@ defmodule LivebookWeb.SettingsLive.FileSystemsComponent do
<.file_system_info file_system={file_system} />
- <.file_system_actions - file_system_id={file_system.id} - default_file_system_id={@default_file_system_id} - /> + <.file_system_actions file_system_id={file_system.id} />
@@ -46,33 +43,12 @@ defmodule LivebookWeb.SettingsLive.FileSystemsComponent do defp file_system_actions(assigns) do ~H"""
- - Default - - <.menu - :if={@default_file_system_id != @file_system_id or @file_system_id != "local"} - id={"file-system-#{@file_system_id}-menu"} - > + <.menu :if={@file_system_id != "local"} id={"file-system-#{@file_system_id}-menu"}> <:toggle> - <.menu_item> - - <.menu_item variant={:danger}>