Lazily lookup file systems when needed for file operations (#2239)

This commit is contained in:
Jonatan Kłosko 2023-09-29 20:24:37 +02:00 committed by GitHub
parent da65a893ce
commit d3f58036eb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
22 changed files with 445 additions and 323 deletions

View file

@ -6,6 +6,14 @@ defprotocol Livebook.FileSystem do
An identifier uniquely identifying the given file system.
Every file system struct is expected have an `:id` field.
The identifier should be computed deterministically based on the
specific resource used as the file system. This ensures that
identifiers persisted in a notebook work for multiple users, as
long as they have a file system using the same resource.
Ths identifier should also include file system type and hub id
(if applicable) in order to avoid conflicts.
"""
@type id :: String.t()
@ -33,13 +41,6 @@ defprotocol Livebook.FileSystem do
@type access :: :read | :write | :read_write | :none
@doc """
Returns a term uniquely identifying the resource used as a file
system.
"""
@spec resource_identifier(t()) :: term()
def resource_identifier(file_system)
@doc """
Returns the file system type.
@ -176,7 +177,7 @@ defprotocol Livebook.FileSystem do
Initializes chunked write to the given file.
Should return the initial state, which is then reduced over in
`write_stream_chunk/3`
`write_stream_chunk/3`.
"""
@spec write_stream_init(t(), path(), keyword()) :: {:ok, state} | {:error, error()}
when state: term()

View file

@ -5,13 +5,19 @@ defmodule Livebook.FileSystem.File do
# the `File` and `Path` core module. Many functions simply delegate
# the work to the underlying file system.
defstruct [:file_system, :path]
defstruct [:file_system_id, :file_system_module, :path, :origin_pid]
alias Livebook.FileSystem
@type t :: %__MODULE__{
file_system: FileSystem.t(),
path: FileSystem.path()
file_system_id: String.t(),
file_system_module: module,
path: FileSystem.path(),
# We cannot just store the node, because when the struct is
# built, we may not yet be in distributed mode. Instead, we
# keep the pid of whatever process created this file system
# and we call node/1 on it whenever needed
origin_pid: pid()
}
@doc """
@ -20,7 +26,7 @@ defmodule Livebook.FileSystem.File do
If no path is given, the default file system one is used.
"""
@spec new(FileSystem.t(), FileSystem.path() | nil) :: t()
def new(file_system, path \\ nil) do
def new(%module{} = file_system, path \\ nil) do
default_path = FileSystem.default_path(file_system)
path =
@ -36,7 +42,12 @@ defmodule Livebook.FileSystem.File do
default_path
end
%__MODULE__{file_system: file_system, path: path}
%__MODULE__{
file_system_id: file_system.id,
file_system_module: module,
path: path,
origin_pid: self()
}
end
@doc """
@ -54,7 +65,16 @@ defmodule Livebook.FileSystem.File do
"""
@spec resource_identifier(t()) :: term()
def resource_identifier(file) do
{FileSystem.resource_identifier(file.file_system), file.path}
# Note that file system id should by definition encapsulate
# information about the underlying resource. We also include node
# if the file system is node-dependent
node =
if FileSystem.type(struct!(file.file_system_module)) == :local do
node(file.origin_pid)
end
{file.file_system_id, node, file.path}
end
@doc """
@ -62,7 +82,7 @@ defmodule Livebook.FileSystem.File do
"""
@spec local?(t()) :: term()
def local?(file) do
FileSystem.type(file.file_system) == :local
FileSystem.type(struct!(file.file_system_module)) == :local
end
@doc """
@ -74,8 +94,10 @@ defmodule Livebook.FileSystem.File do
@spec resolve(t(), String.t()) :: t()
def resolve(file, subject) do
dir = if dir?(file), do: file, else: containing_dir(file)
path = FileSystem.resolve_path(file.file_system, dir.path, subject)
new(file.file_system, path)
path = FileSystem.resolve_path(struct!(file.file_system_module), dir.path, subject)
%{file | path: path}
end
@doc """
@ -124,7 +146,7 @@ defmodule Livebook.FileSystem.File do
|> FileSystem.Utils.ensure_dir_path()
end
new(file.file_system, parent_path)
%{file | path: parent_path}
end
@doc """
@ -140,8 +162,10 @@ defmodule Livebook.FileSystem.File do
def list(file, opts \\ []) do
recursive = Keyword.get(opts, :recursive, false)
with {:ok, paths} <- FileSystem.list(file.file_system, file.path, recursive) do
files = for path <- paths, do: new(file.file_system, path)
with :ok <- maybe_ensure_local(file),
{:ok, file_system} <- do_fetch_file_system(file.file_system_id),
{:ok, paths} <- FileSystem.list(file_system, file.path, recursive) do
files = for path <- paths, do: new(file_system, path)
{:ok, files}
end
end
@ -151,7 +175,10 @@ defmodule Livebook.FileSystem.File do
"""
@spec read(t()) :: {:ok, binary()} | {:error, FileSystem.error()}
def read(file) do
FileSystem.read(file.file_system, file.path)
with :ok <- maybe_ensure_local(file),
{:ok, file_system} <- do_fetch_file_system(file.file_system_id) do
FileSystem.read(file_system, file.path)
end
end
@doc """
@ -159,7 +186,10 @@ defmodule Livebook.FileSystem.File do
"""
@spec write(t(), binary()) :: :ok | {:error, FileSystem.error()}
def write(file, content) do
FileSystem.write(file.file_system, file.path, content)
with :ok <- maybe_ensure_local(file),
{:ok, file_system} <- do_fetch_file_system(file.file_system_id) do
FileSystem.write(file_system, file.path, content)
end
end
@doc """
@ -167,7 +197,10 @@ defmodule Livebook.FileSystem.File do
"""
@spec access(t()) :: {:ok, FileSystem.access()} | {:error, FileSystem.error()}
def access(file) do
FileSystem.access(file.file_system, file.path)
with :ok <- maybe_ensure_local(file),
{:ok, file_system} <- do_fetch_file_system(file.file_system_id) do
FileSystem.access(file_system, file.path)
end
end
@doc """
@ -175,7 +208,10 @@ defmodule Livebook.FileSystem.File do
"""
@spec create_dir(t()) :: :ok | {:error, FileSystem.error()}
def create_dir(file) do
FileSystem.create_dir(file.file_system, file.path)
with :ok <- maybe_ensure_local(file),
{:ok, file_system} <- do_fetch_file_system(file.file_system_id) do
FileSystem.create_dir(file_system, file.path)
end
end
@doc """
@ -183,7 +219,10 @@ defmodule Livebook.FileSystem.File do
"""
@spec remove(t()) :: :ok | {:error, FileSystem.error()}
def remove(file) do
FileSystem.remove(file.file_system, file.path)
with :ok <- maybe_ensure_local(file),
{:ok, file_system} <- do_fetch_file_system(file.file_system_id) do
FileSystem.remove(file_system, file.path)
end
end
@doc """
@ -196,8 +235,12 @@ defmodule Livebook.FileSystem.File do
@spec copy(t(), t()) :: :ok | {:error, FileSystem.error()}
def copy(source, destination)
def copy(%{file_system: file_system} = source, %{file_system: file_system} = destination) do
FileSystem.copy(file_system, source.path, destination.path)
def copy(%{file_system_id: fs_id} = source, %{file_system_id: fs_id} = destination) do
with :ok <- maybe_ensure_local(source),
:ok <- maybe_ensure_local(destination),
{:ok, file_system} <- do_fetch_file_system(fs_id) do
FileSystem.copy(file_system, source.path, destination.path)
end
end
def copy(source, destination) do
@ -236,8 +279,12 @@ defmodule Livebook.FileSystem.File do
@spec rename(t(), t()) :: :ok | {:error, FileSystem.error()}
def rename(source, destination)
def rename(%{file_system: file_system} = source, %{file_system: file_system} = destination) do
FileSystem.rename(file_system, source.path, destination.path)
def rename(%{file_system_id: fs_id} = source, %{file_system_id: fs_id} = destination) do
with :ok <- maybe_ensure_local(source),
:ok <- maybe_ensure_local(destination),
{:ok, file_system} <- do_fetch_file_system(fs_id) do
FileSystem.rename(file_system, source.path, destination.path)
end
end
def rename(source, destination) do
@ -259,7 +306,10 @@ defmodule Livebook.FileSystem.File do
"""
@spec etag_for(t()) :: {:ok, String.t()} | {:error, FileSystem.error()}
def etag_for(file) do
FileSystem.etag_for(file.file_system, file.path)
with :ok <- maybe_ensure_local(file),
{:ok, file_system} <- do_fetch_file_system(file.file_system_id) do
FileSystem.etag_for(file_system, file.path)
end
end
@doc """
@ -267,7 +317,10 @@ defmodule Livebook.FileSystem.File do
"""
@spec exists?(t()) :: {:ok, boolean()} | {:error, FileSystem.error()}
def exists?(file) do
FileSystem.exists?(file.file_system, file.path)
with :ok <- maybe_ensure_local(file),
{:ok, file_system} <- do_fetch_file_system(file.file_system_id) do
FileSystem.exists?(file_system, file.path)
end
end
@doc """
@ -291,13 +344,82 @@ defmodule Livebook.FileSystem.File do
@spec read_stream_into(t(), Collectable.t()) ::
{:ok, Collectable.t()} | {:error, FileSystem.error()}
def read_stream_into(file, collectable) do
FileSystem.read_stream_into(file.file_system, file.path, collectable)
with :ok <- maybe_ensure_local(file),
{:ok, file_system} <- do_fetch_file_system(file.file_system_id) do
FileSystem.read_stream_into(file_system, file.path, collectable)
end
end
@doc """
Checks if the given files use the same file system.
For local file systems also checks if both files actually point to
the same node.
"""
@spec same_file_system?(t(), t()) :: boolean()
def same_file_system?(file1, file2)
def same_file_system?(%{file_system_id: id} = file1, %{file_system_id: id} = file2) do
case {local?(file1), local?(file2)} do
{false, false} -> true
{true, true} -> node(file1.origin_pid) == node(file2.origin_pid)
end
end
def same_file_system?(_file1, _file2), do: false
@doc """
Looks up file system that this file uses.
The file system may not be available in certain cases, for example
when it has been detached.
"""
@spec fetch_file_system(t()) :: {:ok, FileSystem.t()} | {:error, FileSystem.error()}
def fetch_file_system(file) do
do_fetch_file_system(file.file_system_id)
end
defp do_fetch_file_system(file_system_id) do
file_system = Livebook.Hubs.get_file_systems() |> Enum.find(&(&1.id == file_system_id))
if file_system do
{:ok, file_system}
else
{:error,
"could not find file system (id: #{file_system_id}). This means that it has" <>
" been either detached or cannot be accessed from the Hub at the moment"}
end
end
@doc false
def maybe_ensure_local(file) do
if local?(file) do
if node(file.origin_pid) == node() do
:ok
else
{:error, "cannot access local file from a different host"}
end
else
:ok
end
end
end
defimpl Collectable, for: Livebook.FileSystem.File do
def into(%Livebook.FileSystem.File{file_system: file_system, path: path} = file) do
state = file_system |> Livebook.FileSystem.write_stream_init(path, []) |> unwrap!()
def into(%Livebook.FileSystem.File{path: path} = file) do
file_system =
file
|> Livebook.FileSystem.File.fetch_file_system()
|> unwrap!()
file
|> Livebook.FileSystem.File.maybe_ensure_local()
|> unwrap!()
state =
file_system
|> Livebook.FileSystem.write_stream_init(path, [])
|> unwrap!()
collector = fn
state, {:cont, chunk} when is_binary(chunk) ->

View file

@ -1,16 +1,11 @@
defmodule Livebook.FileSystem.Local do
# File system backed by local disk.
defstruct [:origin_pid, :default_path, id: "local"]
defstruct [:default_path, id: "local"]
alias Livebook.FileSystem
@type t :: %__MODULE__{
# We cannot just store the node, because when the struct is
# built, we may not yet be in distributed mode. Instead, we
# keep the pid of whatever process created this file system
# and we call node/1 on it whenever needed
origin_pid: pid(),
default_path: FileSystem.path()
}
@ -31,7 +26,7 @@ defmodule Livebook.FileSystem.Local do
FileSystem.Utils.assert_dir_path!(default_path)
%__MODULE__{origin_pid: self(), default_path: default_path}
%__MODULE__{default_path: default_path}
end
end
@ -40,10 +35,6 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do
@stream_chunk_size_in_bytes 16384
def resource_identifier(file_system) do
{:local_file_system, node(file_system.origin_pid)}
end
def type(_file_system) do
:local
end
@ -55,125 +46,109 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do
def list(file_system, path, recursive) do
FileSystem.Utils.assert_dir_path!(path)
with :ok <- ensure_local(file_system) do
case File.ls(path) do
{:ok, filenames} ->
paths =
Enum.map(filenames, fn name ->
path = Path.join(path, name)
if File.dir?(path), do: path <> "/", else: path
end)
to_traverse =
if recursive do
Enum.filter(paths, &FileSystem.Utils.dir_path?/1)
else
[]
end
Enum.reduce(to_traverse, {:ok, paths}, fn path, result ->
with {:ok, current_paths} <- result,
{:ok, new_paths} <- list(file_system, path, recursive) do
{:ok, current_paths ++ new_paths}
end
case File.ls(path) do
{:ok, filenames} ->
paths =
Enum.map(filenames, fn name ->
path = Path.join(path, name)
if File.dir?(path), do: path <> "/", else: path
end)
{:error, error} ->
FileSystem.Utils.posix_error(error)
end
to_traverse =
if recursive do
Enum.filter(paths, &FileSystem.Utils.dir_path?/1)
else
[]
end
Enum.reduce(to_traverse, {:ok, paths}, fn path, result ->
with {:ok, current_paths} <- result,
{:ok, new_paths} <- list(file_system, path, recursive) do
{:ok, current_paths ++ new_paths}
end
end)
{:error, error} ->
FileSystem.Utils.posix_error(error)
end
end
def read(file_system, path) do
def read(_file_system, path) do
FileSystem.Utils.assert_regular_path!(path)
with :ok <- ensure_local(file_system) do
case File.read(path) do
{:ok, binary} -> {:ok, binary}
{:error, error} -> FileSystem.Utils.posix_error(error)
end
case File.read(path) do
{:ok, binary} -> {:ok, binary}
{:error, error} -> FileSystem.Utils.posix_error(error)
end
end
def write(file_system, path, content) do
def write(_file_system, path, content) do
FileSystem.Utils.assert_regular_path!(path)
dir = Path.dirname(path)
with :ok <- ensure_local(file_system) do
with :ok <- File.mkdir_p(dir),
:ok <- File.write(path, content, [:sync]) do
:ok
else
{:error, error} -> FileSystem.Utils.posix_error(error)
end
with :ok <- File.mkdir_p(dir),
:ok <- File.write(path, content, [:sync]) do
:ok
else
{:error, error} -> FileSystem.Utils.posix_error(error)
end
end
def access(file_system, path) do
with :ok <- ensure_local(file_system) do
case File.stat(path) do
{:ok, stat} -> {:ok, stat.access}
{:error, error} -> FileSystem.Utils.posix_error(error)
end
def access(_file_system, path) do
case File.stat(path) do
{:ok, stat} -> {:ok, stat.access}
{:error, error} -> FileSystem.Utils.posix_error(error)
end
end
def create_dir(file_system, path) do
def create_dir(_file_system, path) do
FileSystem.Utils.assert_dir_path!(path)
with :ok <- ensure_local(file_system) do
case File.mkdir_p(path) do
:ok -> :ok
{:error, error} -> FileSystem.Utils.posix_error(error)
end
case File.mkdir_p(path) do
:ok -> :ok
{:error, error} -> FileSystem.Utils.posix_error(error)
end
end
def remove(file_system, path) do
with :ok <- ensure_local(file_system) do
case File.rm_rf(path) do
{:ok, _paths} -> :ok
{:error, error, _paths} -> FileSystem.Utils.posix_error(error)
end
def remove(_file_system, path) do
case File.rm_rf(path) do
{:ok, _paths} -> :ok
{:error, error, _paths} -> FileSystem.Utils.posix_error(error)
end
end
def copy(file_system, source_path, destination_path) do
def copy(_file_system, source_path, destination_path) do
FileSystem.Utils.assert_same_type!(source_path, destination_path)
containing_dir = Path.dirname(destination_path)
with :ok <- ensure_local(file_system) do
case File.mkdir_p(containing_dir) do
:ok ->
case File.cp_r(source_path, destination_path) do
{:ok, _paths} -> :ok
{:error, error, _path} -> FileSystem.Utils.posix_error(error)
end
case File.mkdir_p(containing_dir) do
:ok ->
case File.cp_r(source_path, destination_path) do
{:ok, _paths} -> :ok
{:error, error, _path} -> FileSystem.Utils.posix_error(error)
end
{:error, error} ->
FileSystem.Utils.posix_error(error)
end
{:error, error} ->
FileSystem.Utils.posix_error(error)
end
end
def rename(file_system, source_path, destination_path) do
def rename(_file_system, source_path, destination_path) do
FileSystem.Utils.assert_same_type!(source_path, destination_path)
with :ok <- ensure_local(file_system) do
if File.exists?(destination_path) do
FileSystem.Utils.posix_error(:eexist)
else
containing_dir = Path.dirname(destination_path)
if File.exists?(destination_path) do
FileSystem.Utils.posix_error(:eexist)
else
containing_dir = Path.dirname(destination_path)
with :ok <- File.mkdir_p(containing_dir),
:ok <- rename_or_move(source_path, destination_path) do
:ok
else
{:error, error} ->
FileSystem.Utils.posix_error(error)
end
with :ok <- File.mkdir_p(containing_dir),
:ok <- rename_or_move(source_path, destination_path) do
:ok
else
{:error, error} ->
FileSystem.Utils.posix_error(error)
end
end
end
@ -190,55 +165,41 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.Local do
end
end
def etag_for(file_system, path) do
with :ok <- ensure_local(file_system) do
case File.stat(path) do
{:ok, stat} ->
%{size: size, mtime: mtime} = stat
hash = {size, mtime} |> :erlang.phash2() |> Integer.to_string(16)
etag = <<?", hash::binary, ?">>
{:ok, etag}
def etag_for(_file_system, path) do
case File.stat(path) do
{:ok, stat} ->
%{size: size, mtime: mtime} = stat
hash = {size, mtime} |> :erlang.phash2() |> Integer.to_string(16)
etag = <<?", hash::binary, ?">>
{:ok, etag}
{:error, error} ->
FileSystem.Utils.posix_error(error)
end
{:error, error} ->
FileSystem.Utils.posix_error(error)
end
end
def exists?(file_system, path) do
with :ok <- ensure_local(file_system) do
if FileSystem.Utils.dir_path?(path) do
{:ok, File.dir?(path)}
else
{:ok, File.exists?(path)}
end
def exists?(_file_system, path) do
if FileSystem.Utils.dir_path?(path) do
{:ok, File.dir?(path)}
else
{:ok, File.exists?(path)}
end
end
def resolve_path(file_system, dir_path, subject) do
def resolve_path(_file_system, dir_path, subject) do
FileSystem.Utils.assert_dir_path!(dir_path)
with :ok <- ensure_local(file_system) do
if subject == "" do
dir_path
else
dir? = FileSystem.Utils.dir_path?(subject) or Path.basename(subject) in [".", ".."]
expanded_path = Path.expand(subject, dir_path)
if dir? do
FileSystem.Utils.ensure_dir_path(expanded_path)
else
expanded_path
end
end
end
end
defp ensure_local(file_system) do
if node(file_system.origin_pid) == node() do
:ok
if subject == "" do
dir_path
else
{:error, "this disk belongs to a different host"}
dir? = FileSystem.Utils.dir_path?(subject) or Path.basename(subject) in [".", ".."]
expanded_path = Path.expand(subject, dir_path)
if dir? do
FileSystem.Utils.ensure_dir_path(expanded_path)
else
expanded_path
end
end
end

View file

@ -104,10 +104,6 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do
alias Livebook.FileSystem
alias Livebook.FileSystem.S3
def resource_identifier(file_system) do
{:s3, file_system.bucket_url}
end
def type(_file_system) do
:global
end

View file

@ -5,7 +5,7 @@ defmodule Livebook.FileSystems do
Returns the type identifier for the given file system.
"""
@spec type(FileSystem.t()) :: String.t()
def type(%FileSystem.S3{}), do: "s3"
def type(%module{}), do: module_to_type(module)
@doc """
Updates file system with the given changes.
@ -38,7 +38,27 @@ defmodule Livebook.FileSystems do
Loads the file system from given type and dumped data.
"""
@spec load(String.t(), map()) :: FileSystem.t()
def load("s3", dumped_data) do
FileSystem.load(%FileSystem.S3{}, dumped_data)
def load(type, dumped_data) do
type
|> type_to_module()
|> struct!()
|> FileSystem.load(dumped_data)
end
@doc """
Returns file system module corresponding to the given type.
"""
@spec type_to_module(String.t()) :: module()
def type_to_module(type)
def type_to_module("local"), do: FileSystem.Local
def type_to_module("s3"), do: FileSystem.S3
@doc """
Returns a serializable type for corresponding to the given file
system module.
"""
@spec module_to_type(module()) :: String.t()
def module_to_type(module)
def module_to_type(FileSystem.Local), do: "local"
def module_to_type(FileSystem.S3), do: "s3"
end

View file

@ -143,7 +143,15 @@ defmodule Livebook.LiveMarkdown.Export do
end
defp file_entry_metadata(%{type: :file, name: name, file: file}) do
%{type: "file", name: name, file: %{file_system_id: file.file_system.id, path: file.path}}
%{
type: "file",
name: name,
file: %{
file_system_id: file.file_system_id,
file_system_type: Livebook.FileSystems.module_to_type(file.file_system_module),
path: file.path
}
}
end
defp file_entry_metadata(%{type: :url, name: name, url: url}) do

View file

@ -412,19 +412,10 @@ defmodule Livebook.LiveMarkdown.Import do
{"file_entries", file_entry_metadata}, {attrs, messages}
when is_list(file_entry_metadata) ->
file_system_by_id =
if Enum.any?(file_entry_metadata, &(&1["type"] == "file")) do
for file_system <- Livebook.Hubs.get_file_systems(),
do: {file_system.id, file_system},
into: %{}
else
%{}
end
{file_entries, file_entry_messages} =
for file_entry_metadata <- file_entry_metadata, reduce: {[], []} do
{file_entries, warnings} ->
case file_entry_metadata_to_attrs(file_entry_metadata, file_system_by_id) do
case file_entry_metadata_to_attrs(file_entry_metadata) do
{:ok, file_entry} -> {[file_entry | file_entries], warnings}
{:error, message} -> {file_entries, [message | warnings]}
end
@ -478,33 +469,37 @@ defmodule Livebook.LiveMarkdown.Import do
end)
end
defp file_entry_metadata_to_attrs(%{"type" => "attachment", "name" => name}, _file_system_by_id) do
defp file_entry_metadata_to_attrs(%{"type" => "attachment", "name" => name}) do
{:ok, %{type: :attachment, name: name}}
end
defp file_entry_metadata_to_attrs(
%{
"type" => "file",
"name" => name,
"file" => %{"file_system_id" => file_system_id, "path" => path}
},
file_system_by_id
) do
if file_system = file_system_by_id[file_system_id] do
file = Livebook.FileSystem.File.new(file_system, path)
{:ok, %{type: :file, name: name, file: file}}
else
{:error, "skipping file #{name}, since it points to an unknown file storage"}
end
defp file_entry_metadata_to_attrs(%{
"type" => "file",
"name" => name,
"file" => %{
"file_system_id" => file_system_id,
"file_system_type" => file_system_type,
"path" => path
}
}) do
file = %Livebook.FileSystem.File{
file_system_id: file_system_id,
file_system_module: Livebook.FileSystems.type_to_module(file_system_type),
path: path,
origin_pid: self()
}
{:ok, %{type: :file, name: name, file: file}}
end
defp file_entry_metadata_to_attrs(
%{"type" => "url", "name" => name, "url" => url},
_file_system_by_id
) do
defp file_entry_metadata_to_attrs(%{"type" => "url", "name" => name, "url" => url}) do
{:ok, %{type: :url, name: name, url: url}}
end
defp file_entry_metadata_to_attrs(_other) do
{:error, "discarding file entry in invalid format"}
end
defp section_metadata_to_attrs(metadata) do
Enum.reduce(metadata, %{}, fn
{"branch_parent_index", parent_idx}, attrs ->

View file

@ -99,14 +99,6 @@ defmodule Livebook.NotebookManager do
GenServer.cast(__MODULE__, {:remove_starred_notebook, file})
end
@doc """
Clears all information about notebooks from the removed file system.
"""
@spec remove_file_system(Livebook.Utils.id()) :: :ok
def remove_file_system(file_system_id) do
GenServer.cast(__MODULE__, {:remove_file_system, file_system_id})
end
@doc """
Updates the tracked notebook name for the given file.
@ -121,6 +113,8 @@ defmodule Livebook.NotebookManager do
@impl true
def init(_opts) do
Livebook.Hubs.subscribe([:file_systems])
{:ok, nil, {:continue, :load_state}}
end
@ -191,14 +185,6 @@ defmodule Livebook.NotebookManager do
{:noreply, state, {:continue, :dump_state}}
end
def handle_cast({:remove_file_system, file_system_id}, state = prev_state) do
recent_notebooks = remove_notebooks_on_file_system(state.recent_notebooks, file_system_id)
starred_notebooks = remove_notebooks_on_file_system(state.starred_notebooks, file_system_id)
state = %{state | recent_notebooks: recent_notebooks, starred_notebooks: starred_notebooks}
broadcast_changes(state, prev_state)
{:noreply, state, {:continue, :dump_state}}
end
def handle_cast({:update_notebook_name, file, name}, state = prev_state) do
recent_notebooks = update_notebook_names(state.recent_notebooks, file, name)
starred_notebooks = update_notebook_names(state.starred_notebooks, file, name)
@ -207,8 +193,19 @@ defmodule Livebook.NotebookManager do
{:noreply, state, {:continue, :dump_state}}
end
@impl true
def handle_info({:file_system_deleted, file_system}, state = prev_state) do
recent_notebooks = remove_notebooks_on_file_system(state.recent_notebooks, file_system.id)
starred_notebooks = remove_notebooks_on_file_system(state.starred_notebooks, file_system.id)
state = %{state | recent_notebooks: recent_notebooks, starred_notebooks: starred_notebooks}
broadcast_changes(state, prev_state)
{:noreply, state, {:continue, :dump_state}}
end
def handle_info(_message, state), do: {:noreply, state}
defp remove_notebooks_on_file_system(notebook_infos, file_system_id) do
Enum.reject(notebook_infos, &(&1.file.file_system.id == file_system_id))
Enum.reject(notebook_infos, &(&1.file.file_system_id == file_system_id))
end
defp update_notebook_names(notebook_infos, file, name) do
@ -243,38 +240,42 @@ defmodule Livebook.NotebookManager do
_ -> %{}
end
file_systems =
Livebook.Storage.all(:file_systems)
|> Enum.sort_by(& &1.bucket_url)
|> Enum.map(fn fields -> Livebook.FileSystems.load(fields.type, fields) end)
file_systems = [Livebook.Config.local_file_system() | file_systems]
file_system_by_id =
for file_system <- file_systems,
do: {file_system.id, file_system},
into: %{}
%{
recent_notebooks: load_notebook_infos(attrs[:recent_notebooks], file_system_by_id),
starred_notebooks: load_notebook_infos(attrs[:starred_notebooks], file_system_by_id)
recent_notebooks: load_notebook_infos(attrs[:recent_notebooks]),
starred_notebooks: load_notebook_infos(attrs[:starred_notebooks])
}
end
defp load_notebook_infos(nil, _file_system_by_id), do: []
defp load_notebook_infos(nil), do: []
defp load_notebook_infos(notebook_infos, file_system_by_id) do
defp load_notebook_infos(notebook_infos) do
for %{file: file, name: name, added_at: added_at} <- notebook_infos,
file = load_file(file, file_system_by_id),
file = load_file(file),
added_at = load_datetime(added_at) do
%{file: file, name: name, added_at: added_at}
end
end
defp load_file(%{file_system_id: file_system_id, path: path}, file_system_by_id) do
if file_system = file_system_by_id[file_system_id] do
%FileSystem.File{file_system: file_system, path: path}
end
defp load_file(%{file_system_id: file_system_id, file_system_type: file_system_type, path: path}) do
%FileSystem.File{
file_system_id: file_system_id,
file_system_module: Livebook.FileSystems.type_to_module(file_system_type),
path: path,
origin_pid: self()
}
end
# TODO: remove on Livebook v0.12
# NotebookManager starts before we run migrations, so we have a
# fallback here instead
defp load_file(%{file_system_id: file_system_id, path: path}) do
file_system_type =
case file_system_id do
"local" -> "local"
"s3-" <> _ -> "s3"
end
load_file(%{file_system_id: "local", path: path, file_system_type: file_system_type})
end
defp load_datetime(datetime) do
@ -301,6 +302,10 @@ defmodule Livebook.NotebookManager do
end
defp dump_file(file) do
%{file_system_id: file.file_system.id, path: file.path}
%{
file_system_id: file.file_system_id,
file_system_type: Livebook.FileSystems.module_to_type(file.file_system_module),
path: file.path
}
end
end

View file

@ -2652,29 +2652,28 @@ defmodule Livebook.Session do
end
defp file_entry_spec_from_file(file) do
if FileSystem.File.local?(file) do
if FileSystem.File.exists?(file) == {:ok, true} do
{:ok, %{type: :local, path: file.path}}
else
{:error, "no file exists at path #{inspect(file.path)}"}
end
else
spec =
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
}
case file.file_system_module do
FileSystem.Local ->
case FileSystem.File.exists?(file) do
{:ok, true} -> {:ok, %{type: :local, path: file.path}}
{:ok, false} -> {:error, "no file exists at path #{inspect(file.path)}"}
{:error, error} -> {:error, error}
end
{:ok, spec}
FileSystem.S3 ->
"/" <> key = file.path
with {:ok, file_system} <- FileSystem.File.fetch_file_system(file) do
{:ok,
%{
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
end
end

View file

@ -172,7 +172,11 @@ defmodule Livebook.Settings do
@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}
default_dir: %{
file_system_id: file.file_system_id,
file_system_type: Livebook.FileSystems.module_to_type(file.file_system_module),
path: file.path
}
)
end
@ -181,11 +185,14 @@ defmodule Livebook.Settings do
"""
@spec default_dir() :: FileSystem.File.t()
def default_dir() do
with {:ok, %{file_system_id: file_system_id, path: path}} <-
with {:ok, %{file_system_id: file_system_id, file_system_type: file_system_type, path: path}} <-
Storage.fetch_key(:settings, "global", :default_dir) do
Livebook.Hubs.get_file_systems()
|> Enum.find(&(&1.id == file_system_id))
|> FileSystem.File.new(path)
%FileSystem.File{
file_system_id: file_system_id,
file_system_module: Livebook.FileSystems.type_to_module(file_system_type),
path: path,
origin_pid: self()
}
else
_ -> FileSystem.File.new(Livebook.Config.local_file_system())
end

View file

@ -281,14 +281,14 @@ defmodule LivebookWeb.FileSelectComponent do
aria-label="switch file storage"
disabled={@file_system_select_disabled}
>
<span><%= file_system_name(@file.file_system) %></span>
<span><%= file_system_name(@file.file_system_module) %></span>
<div class="pl-0.5 flex items-center">
<.remix_icon icon="arrow-down-s-line" class="text-lg leading-none" />
</div>
</button>
</:toggle>
<%= for file_system <- @file_systems do %>
<%= if file_system == @file.file_system do %>
<%= if file_system.id == @file.file_system_id do %>
<.menu_item variant={:selected}>
<button id={"file-system-#{file_system.id}"} role="menuitem">
<.file_system_icon file_system={file_system} />
@ -462,8 +462,11 @@ defmodule LivebookWeb.FileSelectComponent do
end
def handle_event("set_path", %{"path" => path}, socket) do
file_system =
Enum.find(socket.assigns.file_systems, &(&1.id == socket.assigns.file.file_system_id))
file =
socket.assigns.file.file_system
file_system
|> FileSystem.File.new()
|> FileSystem.File.resolve(path)

View file

@ -6,10 +6,10 @@ defmodule LivebookWeb.FileSystemHelpers do
@doc """
Formats the given file system into a short name.
"""
def file_system_name(file_system)
def file_system_name(file_system_module)
def file_system_name(%FileSystem.Local{}), do: "Disk"
def file_system_name(%FileSystem.S3{}), do: "S3"
def file_system_name(FileSystem.Local), do: "Disk"
def file_system_name(FileSystem.S3), do: "S3"
@doc """
Formats the given file system into a descriptive label.

View file

@ -10,14 +10,15 @@ defmodule LivebookWeb.FileSystemWriter do
@behaviour Phoenix.LiveView.UploadWriter
alias Livebook.FileSystem
@impl true
def init(opts) do
file = Keyword.fetch!(opts, :file)
%{file_system: file_system, path: path} = file
with {:ok, write_state} <- Livebook.FileSystem.write_stream_init(file_system, path, []) do
{:ok, %{file: file, write_state: write_state}}
with {:ok, file_system} <- FileSystem.File.fetch_file_system(file),
{:ok, write_state} <- FileSystem.write_stream_init(file_system, file.path, []) do
{:ok, %{file: file, file_system: file_system, write_state: write_state}}
end
end
@ -28,7 +29,7 @@ defmodule LivebookWeb.FileSystemWriter do
@impl true
def write_chunk(chunk, state) do
case Livebook.FileSystem.write_stream_chunk(state.file.file_system, state.write_state, chunk) do
case FileSystem.write_stream_chunk(state.file_system, state.write_state, chunk) do
{:ok, write_state} -> {:ok, %{state | write_state: write_state}}
{:error, message} -> {:error, message, state}
end
@ -36,14 +37,14 @@ defmodule LivebookWeb.FileSystemWriter do
@impl true
def close(state, :done) do
case Livebook.FileSystem.write_stream_finish(state.file.file_system, state.write_state) do
case FileSystem.write_stream_finish(state.file_system, state.write_state) do
:ok -> {:ok, state}
{:error, message} -> {:error, message}
end
end
def close(state, _reason) do
case Livebook.FileSystem.write_stream_halt(state.file.file_system, state.write_state) do
case FileSystem.write_stream_halt(state.file_system, state.write_state) do
:ok -> {:ok, state}
{:error, message} -> {:error, message}
end

View file

@ -108,9 +108,9 @@ defmodule LivebookWeb.Hub.FileSystemFormComponent do
end
defp check_file_system_conectivity(file_system) do
default_dir = FileSystem.File.new(file_system)
default_path = FileSystem.default_path(file_system)
case FileSystem.File.list(default_dir) do
case FileSystem.list(file_system, default_path, false) do
{:ok, _} -> :ok
{:error, message} -> {:error, "Connection test failed: " <> message}
end

View file

@ -1574,7 +1574,17 @@ defmodule LivebookWeb.SessionLive do
socket
end
assigns = %{name: file_entry.name, file: file_entry.file}
file_system_label =
case Livebook.FileSystem.File.fetch_file_system(file_entry.file) do
{:ok, file_system} -> file_system_label(file_system)
_ -> "Not available"
end
assigns = %{
name: file_entry.name,
file: file_entry.file,
file_system_label: file_system_label
}
description = ~H"""
<div>
@ -1583,7 +1593,7 @@ defmodule LivebookWeb.SessionLive do
</div>
<div class="mt-4 flex flex-col gap-2 border border-gray-200 rounded-lg p-4">
<.labeled_text label="Path"><%= @file.path %></.labeled_text>
<.labeled_text label="File system"><%= file_system_label(@file.file_system) %></.labeled_text>
<.labeled_text label="File system"><%= @file_system_label %></.labeled_text>
</div>
"""

View file

@ -12,17 +12,17 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
@impl true
def update(%{event: {:set_file, file, _info}}, socket) do
current_file_system = socket.assigns.draft_file.file_system
current_file = socket.assigns.draft_file
autosave_interval_s =
case file.file_system do
^current_file_system ->
cond do
FileSystem.File.same_file_system?(file, current_file) ->
socket.assigns.new_attrs.autosave_interval_s
%FileSystem.Local{} ->
FileSystem.File.local?(file) ->
Livebook.Notebook.default_autosave_interval_s()
_other ->
true ->
nil
end

View file

@ -35,7 +35,8 @@ defmodule Livebook.FileSystem.FileTest do
describe "local/1" do
test "uses the globally configured local file system instance" do
assert FileSystem.File.local(p("/path")).file_system == Livebook.Config.local_file_system()
assert FileSystem.File.local(p("/path")).file_system_id ==
Livebook.Config.local_file_system().id
end
end
@ -44,7 +45,7 @@ defmodule Livebook.FileSystem.FileTest do
file_system = FileSystem.Local.new()
file = FileSystem.File.new(file_system, p("/dir/nested/file.txt"))
assert %FileSystem.File{file_system: ^file_system, path: p("/other/file.txt")} =
assert %FileSystem.File{path: p("/other/file.txt")} =
FileSystem.File.resolve(file, p("/other/file.txt"))
end
@ -52,7 +53,7 @@ defmodule Livebook.FileSystem.FileTest do
file_system = FileSystem.Local.new()
file = FileSystem.File.new(file_system, p("/dir/nested/file.txt"))
assert %FileSystem.File{file_system: ^file_system, path: p("/dir/other/other_file.txt")} =
assert %FileSystem.File{path: p("/dir/other/other_file.txt")} =
FileSystem.File.resolve(file, "../other/other_file.txt")
end
@ -60,7 +61,7 @@ defmodule Livebook.FileSystem.FileTest do
file_system = FileSystem.Local.new()
dir = FileSystem.File.new(file_system, p("/dir/nested/"))
assert %FileSystem.File{file_system: ^file_system, path: p("/dir/nested/file.txt")} =
assert %FileSystem.File{path: p("/dir/nested/file.txt")} =
FileSystem.File.resolve(dir, "file.txt")
end
@ -68,14 +69,11 @@ defmodule Livebook.FileSystem.FileTest do
file_system = FileSystem.Local.new()
file = FileSystem.File.new(file_system, p("/dir/nested/file.txt"))
assert %FileSystem.File{file_system: ^file_system, path: p("/dir/other/")} =
FileSystem.File.resolve(file, "../other/")
assert %FileSystem.File{path: p("/dir/other/")} = FileSystem.File.resolve(file, "../other/")
assert %FileSystem.File{file_system: ^file_system, path: p("/dir/nested/")} =
FileSystem.File.resolve(file, ".")
assert %FileSystem.File{path: p("/dir/nested/")} = FileSystem.File.resolve(file, ".")
assert %FileSystem.File{file_system: ^file_system, path: p("/dir/")} =
FileSystem.File.resolve(file, "..")
assert %FileSystem.File{path: p("/dir/")} = FileSystem.File.resolve(file, "..")
end
end
@ -274,6 +272,7 @@ defmodule Livebook.FileSystem.FileTest do
%{tmp_dir: tmp_dir} do
bypass = Bypass.open()
s3_fs = build_bypass_file_system(bypass)
persist_file_system(s3_fs)
local_fs = FileSystem.Local.new()
create_tree!(tmp_dir,
@ -298,6 +297,7 @@ defmodule Livebook.FileSystem.FileTest do
%{tmp_dir: tmp_dir} do
bypass = Bypass.open()
s3_fs = build_bypass_file_system(bypass)
persist_file_system(s3_fs)
local_fs = FileSystem.Local.new()
create_tree!(tmp_dir,
@ -352,6 +352,7 @@ defmodule Livebook.FileSystem.FileTest do
%{tmp_dir: tmp_dir} do
bypass = Bypass.open()
s3_fs = build_bypass_file_system(bypass)
persist_file_system(s3_fs)
local_fs = FileSystem.Local.new()
create_tree!(tmp_dir,

View file

@ -1434,7 +1434,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
}
expected_document = """
<!-- livebook:{"file_entries":[{"name":"data.csv","type":"url","url":"https://example.com/data.csv"},{"file":{"file_system_id":"local","path":"#{p("/document.pdf")}"},"name":"document.pdf","type":"file"},{"name":"image.jpg","type":"attachment"}]} -->
<!-- livebook:{"file_entries":[{"name":"data.csv","type":"url","url":"https://example.com/data.csv"},{"file":{"file_system_id":"local","file_system_type":"local","path":"#{p("/document.pdf")}"},"name":"document.pdf","type":"file"},{"name":"image.jpg","type":"attachment"}]} -->
# My Notebook
"""

View file

@ -1221,7 +1221,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
describe "file entries" do
test "imports file entries" do
markdown = """
<!-- livebook:{"file_entries":[{"name":"data.csv","type":"url","url":"https://example.com/data.csv"},{"file":{"file_system_id":"local","path":"#{p("/document.pdf")}"},"name":"document.pdf","type":"file"},{"name":"image.jpg","type":"attachment"}]} -->
<!-- livebook:{"file_entries":[{"name":"data.csv","type":"url","url":"https://example.com/data.csv"},{"file":{"file_system_id":"local","file_system_type":"local","path":"#{p("/document.pdf")}"},"name":"document.pdf","type":"file"},{"name":"image.jpg","type":"attachment"}]} -->
# My Notebook
"""
@ -1235,7 +1235,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
type: :file,
name: "document.pdf",
file: %Livebook.FileSystem.File{
file_system: %Livebook.FileSystem.Local{},
file_system_module: Livebook.FileSystem.Local,
path: p("/document.pdf")
}
},
@ -1244,25 +1244,9 @@ defmodule Livebook.LiveMarkdown.ImportTest do
} = notebook
end
test "skips file entries from unknown file system" do
markdown = """
<!-- livebook:{"file_entries":[{"file":{"file_system_id":"s3-nonexistent","path":"/document.pdf"},"name":"document.pdf","type":"file"}]} -->
# My Notebook
"""
{notebook, messages} = Import.notebook_from_livemd(markdown)
assert %Notebook{file_entries: []} = notebook
assert messages == [
"skipping file document.pdf, since it points to an unknown file storage"
]
end
test "imports :file file entries with quarantine when no stamp is given" do
markdown = """
<!-- livebook:{"file_entries":[{"file":{"file_system_id":"local","path":"#{p("/document.pdf")}"},"name":"document.pdf","type":"file"}]} -->
<!-- livebook:{"file_entries":[{"file":{"file_system_id":"local","file_system_type":"local","path":"#{p("/document.pdf")}"},"name":"document.pdf","type":"file"}]} -->
# My Notebook
"""
@ -1275,7 +1259,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
type: :file,
name: "document.pdf",
file: %Livebook.FileSystem.File{
file_system: %Livebook.FileSystem.Local{},
file_system_module: Livebook.FileSystem.Local,
path: p("/document.pdf")
}
}
@ -1310,7 +1294,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
type: :file,
name: "document.pdf",
file: %Livebook.FileSystem.File{
file_system: %Livebook.FileSystem.Local{},
file_system_module: Livebook.FileSystem.Local,
path: p("/other.pdf")
}
}
@ -1343,7 +1327,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
type: :file,
name: "document2.pdf",
file: %Livebook.FileSystem.File{
file_system: %Livebook.FileSystem.Local{},
file_system_module: Livebook.FileSystem.Local,
path: p("/document.pdf")
}
},
@ -1351,7 +1335,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
type: :file,
name: "document1.pdf",
file: %Livebook.FileSystem.File{
file_system: %Livebook.FileSystem.Local{},
file_system_module: Livebook.FileSystem.Local,
path: p("/document.pdf")
}
}

View file

@ -1655,6 +1655,7 @@ defmodule Livebook.SessionTest do
test "when remote :file replies with the cached path" do
bypass = Bypass.open()
s3_fs = build_bypass_file_system(bypass)
persist_file_system(s3_fs)
bucket_url = s3_fs.bucket_url
Bypass.expect_once(bypass, "GET", "/mybucket/image.jpg", fn conn ->

View file

@ -1786,7 +1786,10 @@ defmodule LivebookWeb.SessionLiveTest do
%{
type: :file,
name: "image.jpg",
file: %FileSystem.File{file_system: %FileSystem.Local{}, path: ^path}
file: %FileSystem.File{
file_system_module: Livebook.FileSystem.Local,
path: ^path
}
}
]
}

View file

@ -119,6 +119,11 @@ defmodule Livebook.HubHelpers do
file_system
end
def persist_file_system(file_system) do
hub = Livebook.Hubs.fetch_hub!(Livebook.Hubs.Personal.id())
:ok = Livebook.Hubs.create_file_system(hub, file_system)
end
defp hub_pid(hub) do
if pid = GenServer.whereis({:via, Registry, {Livebook.HubsRegistry, hub.id}}) do
{:ok, pid}