From d1789f0e05d0808eb8bd44dd947d8ee250aee5a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 7 Jul 2023 12:07:38 +0200 Subject: [PATCH 01/10] Add support for accessing notebook files --- lib/kino/bridge.ex | 8 ++++++++ lib/kino/files.ex | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 lib/kino/files.ex diff --git a/lib/kino/bridge.ex b/lib/kino/bridge.ex index b42d93a8..bc5d9372 100644 --- a/lib/kino/bridge.ex +++ b/lib/kino/bridge.ex @@ -76,6 +76,14 @@ defmodule Kino.Bridge do with {:ok, reply} <- io_request({:livebook_get_file_path, file_ref}), do: reply end + @doc """ + Requests the file path for notebook file with the given name. + """ + @spec get_file_entry_path(String.t()) :: {:ok, term()} | {:error, request_error() | String.t()} + def get_file_entry_path(name) do + with {:ok, reply} <- io_request({:livebook_get_file_entry_path, name}), do: reply + end + @doc """ Associates `object` with `pid`. diff --git a/lib/kino/files.ex b/lib/kino/files.ex new file mode 100644 index 00000000..bd598509 --- /dev/null +++ b/lib/kino/files.ex @@ -0,0 +1,34 @@ +defmodule Kino.Files do + @moduledoc """ + Provides access to notebook files. + """ + + @doc """ + Accesses notebook file with the given name and returns a local path + to ead its contents from. + + This invocation may take a while, in case the file is downloaded + from a URL and is not in the cache. + + > #### File operations {: .info} + > + > You should treat the file as read-only. To avoid unnecessary + > copies the path may potentially be pointing to the original file, + > in which case any write operations would be persisted. This + > behaviour is not always the case, so you should not rely on it + > either. + """ + @spec file_path(String.t()) :: String.t() + def file_path(name) when is_binary(name) do + case Kino.Bridge.get_file_entry_path(name) do + {:ok, path} -> + path + + {:error, message} when is_binary(message) -> + raise message + + {:error, reason} when is_atom(reason) -> + raise "failed to access file path, reason: #{inspect(reason)}" + end + end +end From 9dd47dc64c8c82e4f6d719a73280cd83bd21e356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 7 Jul 2023 15:46:34 +0200 Subject: [PATCH 02/10] Add file spec --- lib/kino/bridge.ex | 8 ++++++++ lib/kino/files.ex | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/kino/bridge.ex b/lib/kino/bridge.ex index bc5d9372..6c542530 100644 --- a/lib/kino/bridge.ex +++ b/lib/kino/bridge.ex @@ -84,6 +84,14 @@ defmodule Kino.Bridge do with {:ok, reply} <- io_request({:livebook_get_file_entry_path, name}), do: reply end + @doc """ + Requests the file spec for notebook file with the given name. + """ + @spec get_file_entry_spec(String.t()) :: {:ok, term()} | {:error, request_error() | String.t()} + def get_file_entry_spec(name) do + with {:ok, reply} <- io_request({:livebook_get_file_entry_spec, name}), do: reply + end + @doc """ Associates `object` with `pid`. diff --git a/lib/kino/files.ex b/lib/kino/files.ex index bd598509..0b081b6d 100644 --- a/lib/kino/files.ex +++ b/lib/kino/files.ex @@ -31,4 +31,37 @@ defmodule Kino.Files do raise "failed to access file path, reason: #{inspect(reason)}" end end + + @typep file_spec :: + %{ + type: :local, + path: String.t() + } + | %{ + type: :url, + url: String.t() + } + | %{ + type: :s3, + bucket_url: String.t(), + region: String.t(), + access_key_id: String.t(), + secret_access_key: String.t(), + key: String.t() + } + + @doc false + @spec file_spec(String.t()) :: file_spec() + def file_spec(name) do + case Kino.Bridge.get_file_entry_spec(name) do + {:ok, spec} -> + spec + + {:error, message} when is_binary(message) -> + raise message + + {:error, reason} when is_atom(reason) -> + raise "failed to access file spec, reason: #{inspect(reason)}" + end + end end From 9fa2d3fda5efd2f84bdbf152cac8c9e0757b3cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 7 Jul 2023 15:58:11 +0200 Subject: [PATCH 03/10] Kino.Files -> Kino.FS --- lib/kino/{files.ex => fs.ex} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename lib/kino/{files.ex => fs.ex} (98%) diff --git a/lib/kino/files.ex b/lib/kino/fs.ex similarity index 98% rename from lib/kino/files.ex rename to lib/kino/fs.ex index 0b081b6d..37eba702 100644 --- a/lib/kino/files.ex +++ b/lib/kino/fs.ex @@ -1,4 +1,4 @@ -defmodule Kino.Files do +defmodule Kino.FS do @moduledoc """ Provides access to notebook files. """ From 920dceb1f8a54f6c6bbc21f294505d61aea26bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Mon, 17 Jul 2023 17:53:04 +0200 Subject: [PATCH 04/10] Add forbidden error --- lib/kino/bridge.ex | 6 ++++-- lib/kino/fs.ex | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/kino/bridge.ex b/lib/kino/bridge.ex index 6c542530..ced289d2 100644 --- a/lib/kino/bridge.ex +++ b/lib/kino/bridge.ex @@ -79,7 +79,8 @@ defmodule Kino.Bridge do @doc """ Requests the file path for notebook file with the given name. """ - @spec get_file_entry_path(String.t()) :: {:ok, term()} | {:error, request_error() | String.t()} + @spec get_file_entry_path(String.t()) :: + {:ok, term()} | {:error, request_error() | :forbidden | String.t()} def get_file_entry_path(name) do with {:ok, reply} <- io_request({:livebook_get_file_entry_path, name}), do: reply end @@ -87,7 +88,8 @@ defmodule Kino.Bridge do @doc """ Requests the file spec for notebook file with the given name. """ - @spec get_file_entry_spec(String.t()) :: {:ok, term()} | {:error, request_error() | String.t()} + @spec get_file_entry_spec(String.t()) :: + {:ok, term()} | {:error, request_error() | :forbidden | String.t()} def get_file_entry_spec(name) do with {:ok, reply} <- io_request({:livebook_get_file_entry_spec, name}), do: reply end diff --git a/lib/kino/fs.ex b/lib/kino/fs.ex index 37eba702..3011e4ea 100644 --- a/lib/kino/fs.ex +++ b/lib/kino/fs.ex @@ -3,6 +3,19 @@ defmodule Kino.FS do Provides access to notebook files. """ + defmodule ForbiddenError do + @moduledoc """ + Exception raised when access to a notebook file is forbidden. + """ + + defexception [:name] + + @impl true + def message(exception) do + "forbidden access to file #{inspect(exception.name)}" + end + end + @doc """ Accesses notebook file with the given name and returns a local path to ead its contents from. @@ -24,6 +37,9 @@ defmodule Kino.FS do {:ok, path} -> path + {:error, :forbidden} -> + raise ForbiddenError, name: name + {:error, message} when is_binary(message) -> raise message @@ -57,6 +73,9 @@ defmodule Kino.FS do {:ok, spec} -> spec + {:error, :forbidden} -> + raise ForbiddenError, name: name + {:error, message} when is_binary(message) -> raise message From d4f0dee05f6feca6b29b17e7fbac772eaeb50fc7 Mon Sep 17 00:00:00 2001 From: Philip Sampaio Date: Tue, 22 Aug 2023 17:07:51 -0300 Subject: [PATCH 05/10] Convert file spec to FSS entry --- lib/kino/fs.ex | 34 ++++++++++++++++++++-- mix.exs | 1 + mix.lock | 1 + test/kino/fs_test.exs | 68 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 test/kino/fs_test.exs diff --git a/lib/kino/fs.ex b/lib/kino/fs.ex index 3011e4ea..bd81e09a 100644 --- a/lib/kino/fs.ex +++ b/lib/kino/fs.ex @@ -66,12 +66,14 @@ defmodule Kino.FS do key: String.t() } + @typep fss_entry :: FSS.Local.Entry.t() | FSS.S3.Entry.t() | FSS.HTTP.Entry.t() + @doc false - @spec file_spec(String.t()) :: file_spec() + @spec file_spec(String.t()) :: fss_entry() def file_spec(name) do case Kino.Bridge.get_file_entry_spec(name) do {:ok, spec} -> - spec + file_spec_to_fss(spec) {:error, :forbidden} -> raise ForbiddenError, name: name @@ -83,4 +85,32 @@ defmodule Kino.FS do raise "failed to access file spec, reason: #{inspect(reason)}" end end + + @doc false + @spec file_spec_to_fss(file_spec()) :: + fss_entry() + def file_spec_to_fss(%{type: :local} = file_spec) do + %FSS.Local.Entry{path: file_spec.path} + end + + def file_spec_to_fss(%{type: :url} = file_spec) do + case FSS.HTTP.parse(file_spec.url) do + {:ok, entry} -> entry + {:error, error} -> raise error + end + end + + def file_spec_to_fss(%{type: :s3} = file_spec) do + case FSS.S3.parse("s3:///" <> file_spec.key, + config: [ + region: file_spec.region, + endpoint: file_spec.bucket_url, + access_key_id: file_spec.access_key_id, + secret_access_key: file_spec.secret_access_key + ] + ) do + {:ok, entry} -> entry + {:error, error} -> raise error + end + end end diff --git a/mix.exs b/mix.exs index 54d34693..a66eebe5 100644 --- a/mix.exs +++ b/mix.exs @@ -32,6 +32,7 @@ defmodule Kino.MixProject do defp deps do [ {:table, "~> 0.1.2"}, + {:fss, github: "elixir-explorer/fss", branch: "main"}, {:nx, "~> 0.1", optional: true}, {:ex_doc, "~> 0.28", only: :dev, runtime: false} ] diff --git a/mix.lock b/mix.lock index 52764b49..2aa99046 100644 --- a/mix.lock +++ b/mix.lock @@ -2,6 +2,7 @@ "complex": {:hex, :complex, "0.4.2", "923e5db0be13dbb3ea00cf8459d9f75f3afdd9ff5a82742ded21064330d28273", [:mix], [], "hexpm", "069a085ef820ce675a2619fd125b963ff4514af2102c7f7d7965128e5ec0a429"}, "earmark_parser": {:hex, :earmark_parser, "1.4.31", "a93921cdc6b9b869f519213d5bc79d9e218ba768d7270d46fdcf1c01bacff9e2", [:mix], [], "hexpm", "317d367ee0335ef037a87e46c91a2269fef6306413f731e8ec11fc45a7efd059"}, "ex_doc": {:hex, :ex_doc, "0.29.4", "6257ecbb20c7396b1fe5accd55b7b0d23f44b6aa18017b415cb4c2b91d997729", [:mix], [{:earmark_parser, "~> 1.4.31", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "2c6699a737ae46cb61e4ed012af931b57b699643b24dabe2400a8168414bc4f5"}, + "fss": {:git, "https://github.com/elixir-explorer/fss.git", "bf54b55612d12aa172aa7f75d0555359a20bfab1", [branch: "main"]}, "makeup": {:hex, :makeup, "1.1.0", "6b67c8bc2882a6b6a445859952a602afc1a41c2e08379ca057c0f525366fc3ca", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "0a45ed501f4a8897f580eabf99a2e5234ea3e75a4373c8a52824f6e873be57a6"}, "makeup_elixir": {:hex, :makeup_elixir, "0.16.1", "cc9e3ca312f1cfeccc572b37a09980287e243648108384b97ff2b76e505c3555", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "e127a341ad1b209bd80f7bd1620a15693a9908ed780c3b763bccf7d200c767c6"}, "makeup_erlang": {:hex, :makeup_erlang, "0.1.1", "3fcb7f09eb9d98dc4d208f49cc955a34218fc41ff6b84df7c75b3e6e533cc65f", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "174d0809e98a4ef0b3309256cbf97101c6ec01c4ab0b23e926a9e17df2077cbb"}, diff --git a/test/kino/fs_test.exs b/test/kino/fs_test.exs new file mode 100644 index 00000000..d2958491 --- /dev/null +++ b/test/kino/fs_test.exs @@ -0,0 +1,68 @@ +defmodule Kino.FSTest do + use ExUnit.Case, async: true + + describe "file_spec_to_fss/1" do + test "returns a file spec" do + path = "/home/bob/file.txt" + assert %FSS.Local.Entry{path: ^path} = Kino.FS.file_spec_to_fss(%{type: :local, path: path}) + end + + test "returns an HTTP FSS entry" do + url = "https://example.com/file.txt" + + assert %FSS.HTTP.Entry{url: ^url, config: %FSS.HTTP.Config{headers: []}} = + Kino.FS.file_spec_to_fss(%{type: :url, url: url}) + end + + test "returns a S3 FSS entry" do + bucket_url = "https://s3.us-west-1.amazonaws.com/my-bucket" + + s3_spec = %{ + type: :s3, + bucket_url: bucket_url, + region: "us-west-1", + access_key_id: "access-key-1", + secret_access_key: "secret-access-key-1", + key: "file" + } + + assert %FSS.S3.Entry{} = s3 = Kino.FS.file_spec_to_fss(s3_spec) + + assert s3.key == s3_spec.key + + assert s3.config.region == s3_spec.region + assert s3.config.endpoint == bucket_url + assert s3.config.access_key_id == s3_spec.access_key_id + assert s3.config.secret_access_key == s3_spec.secret_access_key + assert s3.config.bucket == nil + end + + test "raises an error in case s3 file_spec has something nil" do + s3_spec = %{ + type: :s3, + bucket_url: nil, + region: "us-west-1", + access_key_id: "access-key-1", + secret_access_key: "secret-access-key-1", + key: "file" + } + + assert_raise ArgumentError, "endpoint is required when bucket is nil", fn -> + Kino.FS.file_spec_to_fss(s3_spec) + end + + bucket_url = "https://s3.us-west-1.amazonaws.com/my-bucket" + + s3_spec = + s3_spec + |> Map.replace!(:bucket_url, bucket_url) + |> Map.replace!(:access_key_id, nil) + + assert_raise ArgumentError, + "missing :access_key_id for FSS.S3 (set the key or the AWS_ACCESS_KEY_ID env var)", + fn -> + Kino.FS.file_spec_to_fss(s3_spec) + end + end + end +end From 776675840ce6818c1e9503ae33d882940002879c Mon Sep 17 00:00:00 2001 From: Philip Sampaio Date: Tue, 22 Aug 2023 18:07:55 -0300 Subject: [PATCH 06/10] Update FSS --- mix.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.lock b/mix.lock index 2aa99046..e349ea35 100644 --- a/mix.lock +++ b/mix.lock @@ -2,7 +2,7 @@ "complex": {:hex, :complex, "0.4.2", "923e5db0be13dbb3ea00cf8459d9f75f3afdd9ff5a82742ded21064330d28273", [:mix], [], "hexpm", "069a085ef820ce675a2619fd125b963ff4514af2102c7f7d7965128e5ec0a429"}, "earmark_parser": {:hex, :earmark_parser, "1.4.31", "a93921cdc6b9b869f519213d5bc79d9e218ba768d7270d46fdcf1c01bacff9e2", [:mix], [], "hexpm", "317d367ee0335ef037a87e46c91a2269fef6306413f731e8ec11fc45a7efd059"}, "ex_doc": {:hex, :ex_doc, "0.29.4", "6257ecbb20c7396b1fe5accd55b7b0d23f44b6aa18017b415cb4c2b91d997729", [:mix], [{:earmark_parser, "~> 1.4.31", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "2c6699a737ae46cb61e4ed012af931b57b699643b24dabe2400a8168414bc4f5"}, - "fss": {:git, "https://github.com/elixir-explorer/fss.git", "bf54b55612d12aa172aa7f75d0555359a20bfab1", [branch: "main"]}, + "fss": {:git, "https://github.com/elixir-explorer/fss.git", "19ed6ce8359e9790e818b732ba8d552aaf36e029", [branch: "main"]}, "makeup": {:hex, :makeup, "1.1.0", "6b67c8bc2882a6b6a445859952a602afc1a41c2e08379ca057c0f525366fc3ca", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "0a45ed501f4a8897f580eabf99a2e5234ea3e75a4373c8a52824f6e873be57a6"}, "makeup_elixir": {:hex, :makeup_elixir, "0.16.1", "cc9e3ca312f1cfeccc572b37a09980287e243648108384b97ff2b76e505c3555", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "e127a341ad1b209bd80f7bd1620a15693a9908ed780c3b763bccf7d200c767c6"}, "makeup_erlang": {:hex, :makeup_erlang, "0.1.1", "3fcb7f09eb9d98dc4d208f49cc955a34218fc41ff6b84df7c75b3e6e533cc65f", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "174d0809e98a4ef0b3309256cbf97101c6ec01c4ab0b23e926a9e17df2077cbb"}, From 0f4e94f43ec6f7d098daec87d36aab5d3349c444 Mon Sep 17 00:00:00 2001 From: Philip Sampaio Date: Tue, 22 Aug 2023 18:30:12 -0300 Subject: [PATCH 07/10] Apply suggestions from the pull request --- lib/kino/fs.ex | 31 +++------------------- test/kino/fs_test.exs | 62 +++++++++++++++++++++++++++++++------------ 2 files changed, 49 insertions(+), 44 deletions(-) diff --git a/lib/kino/fs.ex b/lib/kino/fs.ex index bd81e09a..89b65c8f 100644 --- a/lib/kino/fs.ex +++ b/lib/kino/fs.ex @@ -48,28 +48,8 @@ defmodule Kino.FS do end end - @typep file_spec :: - %{ - type: :local, - path: String.t() - } - | %{ - type: :url, - url: String.t() - } - | %{ - type: :s3, - bucket_url: String.t(), - region: String.t(), - access_key_id: String.t(), - secret_access_key: String.t(), - key: String.t() - } - - @typep fss_entry :: FSS.Local.Entry.t() | FSS.S3.Entry.t() | FSS.HTTP.Entry.t() - @doc false - @spec file_spec(String.t()) :: fss_entry() + @spec file_spec(String.t()) :: FSS.entry() def file_spec(name) do case Kino.Bridge.get_file_entry_spec(name) do {:ok, spec} -> @@ -86,21 +66,18 @@ defmodule Kino.FS do end end - @doc false - @spec file_spec_to_fss(file_spec()) :: - fss_entry() - def file_spec_to_fss(%{type: :local} = file_spec) do + defp file_spec_to_fss(%{type: :local} = file_spec) do %FSS.Local.Entry{path: file_spec.path} end - def file_spec_to_fss(%{type: :url} = file_spec) do + defp file_spec_to_fss(%{type: :url} = file_spec) do case FSS.HTTP.parse(file_spec.url) do {:ok, entry} -> entry {:error, error} -> raise error end end - def file_spec_to_fss(%{type: :s3} = file_spec) do + defp file_spec_to_fss(%{type: :s3} = file_spec) do case FSS.S3.parse("s3:///" <> file_spec.key, config: [ region: file_spec.region, diff --git a/test/kino/fs_test.exs b/test/kino/fs_test.exs index d2958491..253a0d0a 100644 --- a/test/kino/fs_test.exs +++ b/test/kino/fs_test.exs @@ -1,68 +1,96 @@ defmodule Kino.FSTest do use ExUnit.Case, async: true - describe "file_spec_to_fss/1" do + describe "file_spec/1" do test "returns a file spec" do + name = "file.txt" path = "/home/bob/file.txt" - assert %FSS.Local.Entry{path: ^path} = Kino.FS.file_spec_to_fss(%{type: :local, path: path}) + spec = %{type: :local, path: path} + + configure_gl_with_reply({:livebook_get_file_entry_spec, name}, {:ok, spec}) + + assert %FSS.Local.Entry{path: ^path} = Kino.FS.file_spec(name) end test "returns an HTTP FSS entry" do - url = "https://example.com/file.txt" + name = "remote-file.txt" + url = "https://example.com/remote-file.txt" + spec = %{type: :url, url: url} + + configure_gl_with_reply({:livebook_get_file_entry_spec, name}, {:ok, spec}) assert %FSS.HTTP.Entry{url: ^url, config: %FSS.HTTP.Config{headers: []}} = - Kino.FS.file_spec_to_fss(%{type: :url, url: url}) + Kino.FS.file_spec(name) end test "returns a S3 FSS entry" do + name = "file-from-s3.txt" bucket_url = "https://s3.us-west-1.amazonaws.com/my-bucket" - s3_spec = %{ + spec = %{ type: :s3, bucket_url: bucket_url, region: "us-west-1", access_key_id: "access-key-1", secret_access_key: "secret-access-key-1", - key: "file" + key: "file-from-s3.txt" } - assert %FSS.S3.Entry{} = s3 = Kino.FS.file_spec_to_fss(s3_spec) + configure_gl_with_reply({:livebook_get_file_entry_spec, name}, {:ok, spec}) - assert s3.key == s3_spec.key + assert %FSS.S3.Entry{} = s3 = Kino.FS.file_spec(name) - assert s3.config.region == s3_spec.region + assert s3.key == spec.key + + assert s3.config.region == spec.region assert s3.config.endpoint == bucket_url - assert s3.config.access_key_id == s3_spec.access_key_id - assert s3.config.secret_access_key == s3_spec.secret_access_key + assert s3.config.access_key_id == spec.access_key_id + assert s3.config.secret_access_key == spec.secret_access_key assert s3.config.bucket == nil end test "raises an error in case s3 file_spec has something nil" do - s3_spec = %{ + name = "file-from-s3.txt" + + spec = %{ type: :s3, bucket_url: nil, region: "us-west-1", access_key_id: "access-key-1", secret_access_key: "secret-access-key-1", - key: "file" + key: name } + configure_gl_with_reply({:livebook_get_file_entry_spec, name}, {:ok, spec}) + assert_raise ArgumentError, "endpoint is required when bucket is nil", fn -> - Kino.FS.file_spec_to_fss(s3_spec) + Kino.FS.file_spec(name) end bucket_url = "https://s3.us-west-1.amazonaws.com/my-bucket" - s3_spec = - s3_spec + spec = + spec |> Map.replace!(:bucket_url, bucket_url) |> Map.replace!(:access_key_id, nil) + configure_gl_with_reply({:livebook_get_file_entry_spec, name}, {:ok, spec}) + assert_raise ArgumentError, "missing :access_key_id for FSS.S3 (set the key or the AWS_ACCESS_KEY_ID env var)", fn -> - Kino.FS.file_spec_to_fss(s3_spec) + Kino.FS.file_spec(name) end end end + + defp configure_gl_with_reply(request, reply) do + gl = + spawn(fn -> + assert_receive {:io_request, from, reply_as, ^request} + send(from, {:io_reply, reply_as, reply}) + end) + + Process.group_leader(self(), gl) + end end From 7866cfbdd80eb9e457ec78568cf808285be86cd3 Mon Sep 17 00:00:00 2001 From: Philip Sampaio Date: Tue, 22 Aug 2023 18:31:35 -0300 Subject: [PATCH 08/10] Use FSS.Local.from_path/1 --- lib/kino/fs.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kino/fs.ex b/lib/kino/fs.ex index 89b65c8f..0cae4710 100644 --- a/lib/kino/fs.ex +++ b/lib/kino/fs.ex @@ -67,7 +67,7 @@ defmodule Kino.FS do end defp file_spec_to_fss(%{type: :local} = file_spec) do - %FSS.Local.Entry{path: file_spec.path} + FSS.Local.from_path(file_spec.path) end defp file_spec_to_fss(%{type: :url} = file_spec) do From 0c3c7f4b75bcd4c157c297026dacf5a97d1f4137 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Tue, 22 Aug 2023 23:35:17 +0200 Subject: [PATCH 09/10] Apply suggestions from code review --- lib/kino/fs.ex | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/kino/fs.ex b/lib/kino/fs.ex index 0cae4710..d9cbc800 100644 --- a/lib/kino/fs.ex +++ b/lib/kino/fs.ex @@ -18,7 +18,7 @@ defmodule Kino.FS do @doc """ Accesses notebook file with the given name and returns a local path - to ead its contents from. + to read its contents from. This invocation may take a while, in case the file is downloaded from a URL and is not in the cache. @@ -48,7 +48,14 @@ defmodule Kino.FS do end end - @doc false + @doc """ + Accesses notebook file with the given name and returns a specification + of the file location. + + This does not copy any files and moves the responsibility of reading + the file to the caller. If you need to read a file directly, use + `file_path/1`. + """ @spec file_spec(String.t()) :: FSS.entry() def file_spec(name) do case Kino.Bridge.get_file_entry_spec(name) do From 5ba9f98173a374f7838097279ad69c60afba56a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Tue, 22 Aug 2023 23:36:26 +0200 Subject: [PATCH 10/10] Apply suggestions from code review --- lib/kino/fs.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kino/fs.ex b/lib/kino/fs.ex index d9cbc800..9d03fa44 100644 --- a/lib/kino/fs.ex +++ b/lib/kino/fs.ex @@ -51,7 +51,7 @@ defmodule Kino.FS do @doc """ Accesses notebook file with the given name and returns a specification of the file location. - + This does not copy any files and moves the responsibility of reading the file to the caller. If you need to read a file directly, use `file_path/1`.