Final adjustments to env vars (#1418)

This commit is contained in:
Alexandre de Souza 2022-09-16 20:37:01 -03:00 committed by GitHub
parent 7cd7defa6c
commit 2766cf3256
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 170 additions and 96 deletions

View file

@ -218,7 +218,8 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServer do
Keyword.get(opts, :smart_cell_definitions_module, Kino.SmartCell),
extra_smart_cell_definitions: Keyword.get(opts, :extra_smart_cell_definitions, []),
memory_timer_ref: nil,
last_evaluator: nil
last_evaluator: nil,
initial_path: System.get_env("PATH", "")
}}
end
@ -477,13 +478,23 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServer do
end
def handle_cast({:put_system_envs, envs}, state) do
System.put_env(envs)
envs
|> Enum.map(fn
{"PATH", path} -> {"PATH", state.initial_path <> os_path_separator() <> path}
other -> other
end)
|> System.put_env()
{:noreply, state}
end
def handle_cast({:delete_system_envs, names}, state) do
Enum.each(names, &System.delete_env/1)
names
|> Enum.map(fn
"PATH" -> {"PATH", state.initial_path}
name -> {name, nil}
end)
|> System.put_env()
{:noreply, state}
end
@ -644,4 +655,11 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServer do
end)
end)
end
defp os_path_separator() do
case :os.type() do
{:win32, _} -> ";"
_ -> ":"
end
end
end

View file

@ -1125,7 +1125,7 @@ defmodule Livebook.Session do
def handle_info({:env_var_set, env_var}, state) do
if Runtime.connected?(state.data.runtime) do
Runtime.put_system_envs(state.data.runtime, %{env_var.key => env_var.value})
Runtime.put_system_envs(state.data.runtime, [{env_var.name, env_var.value}])
end
{:noreply, state}
@ -1133,7 +1133,7 @@ defmodule Livebook.Session do
def handle_info({:env_var_unset, env_var}, state) do
if Runtime.connected?(state.data.runtime) do
Runtime.delete_system_envs(state.data.runtime, [env_var.key])
Runtime.delete_system_envs(state.data.runtime, [env_var.name])
end
{:noreply, state}
@ -1576,7 +1576,7 @@ defmodule Livebook.Session do
end
defp set_runtime_env_vars(state) do
env_vars = Enum.map(Livebook.Settings.fetch_env_vars(), &{&1.key, &1.value})
env_vars = Enum.map(Livebook.Settings.fetch_env_vars(), &{&1.name, &1.value})
Runtime.put_system_envs(state.data.runtime, env_vars)
end

View file

@ -181,7 +181,7 @@ defmodule Livebook.Settings do
defp save_env_var(env_var) do
attributes = env_var |> Map.from_struct() |> Map.to_list()
with :ok <- storage().insert(:env_vars, env_var.key, attributes),
with :ok <- storage().insert(:env_vars, env_var.name, attributes),
:ok <- broadcast_env_vars_change({:env_var_set, env_var}) do
{:ok, env_var}
end

View file

@ -4,20 +4,20 @@ defmodule Livebook.Settings.EnvVar do
import Ecto.Changeset
@type t :: %__MODULE__{
key: String.t(),
name: String.t(),
value: String.t()
}
@primary_key {:key, :string, autogenerate: false}
@primary_key {:name, :string, autogenerate: false}
embedded_schema do
field :value, :string
end
def changeset(env_var, attrs \\ %{}) do
env_var
|> cast(attrs, [:key, :value])
|> update_change(:key, &String.upcase/1)
|> validate_format(:key, ~r/^(?!LB_)\w+$/, message: "cannot start with the LB_ prefix")
|> validate_required([:key, :value])
|> cast(attrs, [:name, :value])
|> update_change(:name, &String.upcase/1)
|> validate_format(:name, ~r/^(?!LB_)\w+$/, message: "cannot start with the LB_ prefix")
|> validate_required([:name, :value])
end
end

View file

@ -41,11 +41,11 @@ defmodule LivebookWeb.EnvVarComponent do
spellcheck="false"
>
<div class="flex flex-col space-y-4">
<.input_wrapper form={f} field={:key} class="flex flex-col space-y-1">
<.input_wrapper form={f} field={:name} class="flex flex-col space-y-1">
<div class="input-label">
Key <span class="text-xs text-gray-500">(alphanumeric and underscore)</span>
Name <span class="text-xs text-gray-500">(alphanumeric and underscore)</span>
</div>
<%= text_input(f, :key, class: "input", autofocus: @operation == :new) %>
<%= text_input(f, :name, class: "input", autofocus: @operation == :new) %>
</.input_wrapper>
<.input_wrapper form={f} field={:value} class="flex flex-col space-y-1">
<div class="input-label">Value</div>

View file

@ -3,14 +3,22 @@ defmodule LivebookWeb.EnvVarsComponent do
@impl true
def render(assigns) do
assigns = assign_new(assigns, :target, fn -> nil end)
assigns =
assigns
|> assign_new(:target, fn -> nil end)
|> assign_new(:edit_label, fn -> "Edit" end)
~H"""
<div id={@id} class="flex flex-col space-y-4">
<div class="flex flex-col space-y-4">
<%= for env_var <- @env_vars do %>
<div class="flex items-center justify-between border border-gray-200 rounded-lg p-4">
<.env_var_info socket={@socket} env_var={env_var} target={@target} />
<.env_var_info
socket={@socket}
env_var={env_var}
edit_label={@edit_label}
target={@target}
/>
</div>
<% end %>
</div>
@ -29,13 +37,13 @@ defmodule LivebookWeb.EnvVarsComponent do
~H"""
<div class="grid grid-cols-1 md:grid-cols-2 w-full">
<div class="place-content-start">
<.labeled_text label="Key">
<%= @env_var.key %>
<.labeled_text label="Name">
<%= @env_var.name %>
</.labeled_text>
</div>
<div class="flex items-center place-content-end">
<.menu id={"env-var-#{@env_var.key}-menu"}>
<.menu id={"env-var-#{@env_var.name}-menu"}>
<:toggle>
<button class="icon-button" aria-label="open session menu" type="button">
<.remix_icon icon="more-2-fill" class="text-xl" />
@ -43,23 +51,23 @@ defmodule LivebookWeb.EnvVarsComponent do
</:toggle>
<:content>
<button
id={"env-var-#{@env_var.key}-edit"}
id={"env-var-#{@env_var.name}-edit"}
type="button"
phx-click={JS.push("edit_env_var", value: %{env_var: @env_var.key})}
phx-click={JS.push("edit_env_var", value: %{env_var: @env_var.name})}
phx-target={@target}
role="menuitem"
class="menu-item text-gray-600"
>
<.remix_icon icon="file-edit-line" />
<span class="font-medium">Edit</span>
<span class="font-medium"><%= @edit_label %></span>
</button>
<button
id={"env-var-#{@env_var.key}-delete"}
id={"env-var-#{@env_var.name}-delete"}
type="button"
phx-click={
with_confirm(
JS.push("delete_env_var", value: %{env_var: @env_var.key}),
title: "Delete #{@env_var.key}",
JS.push("delete_env_var", value: %{env_var: @env_var.name}),
title: "Delete #{@env_var.name}",
description: "Are you sure you want to delete environment variable?",
confirm_text: "Delete",
confirm_icon: "delete-bin-6-line"

View file

@ -11,8 +11,8 @@ defmodule LivebookWeb.Hub.Edit.FlyComponent do
env_vars = env_vars_from_secrets(app["secrets"])
env_var =
if key = assigns.env_var_id do
Enum.find(env_vars, &(&1.key == key))
if name = assigns.env_var_id do
Enum.find(env_vars, &(&1.name == name))
end
{:ok,
@ -28,7 +28,7 @@ defmodule LivebookWeb.Hub.Edit.FlyComponent do
defp env_vars_from_secrets(secrets) do
for secret <- secrets do
%Livebook.Settings.EnvVar{key: secret["name"]}
%Livebook.Settings.EnvVar{name: secret["name"]}
end
end
@ -105,6 +105,7 @@ defmodule LivebookWeb.Hub.Edit.FlyComponent do
env_vars={@env_vars}
return_to={Routes.hub_path(@socket, :edit, @hub.id)}
add_env_var_path={Routes.hub_path(@socket, :add_env_var, @hub.id)}
edit_label="Replace"
target={@myself}
/>
</div>
@ -157,7 +158,8 @@ defmodule LivebookWeb.Hub.Edit.FlyComponent do
# EnvVar component callbacks
def handle_event("save_env_var", %{"env_var" => attrs}, socket) do
{env_operation, attrs} = Map.pop!(attrs, "operation")
env_operation = attrs["operation"]
attrs = %{"key" => attrs["name"], "value" => attrs["value"]}
case FlyClient.put_secrets(socket.assigns.hub, [attrs]) do
{:ok, _} ->

View file

@ -357,7 +357,7 @@ defmodule LivebookWeb.SettingsLive do
end
def handle_info({:env_var_set, env_var}, socket) do
idx = Enum.find_index(socket.assigns.env_vars, &(&1.key == env_var.key))
idx = Enum.find_index(socket.assigns.env_vars, &(&1.name == env_var.name))
env_vars =
if idx,
@ -368,7 +368,7 @@ defmodule LivebookWeb.SettingsLive do
end
def handle_info({:env_var_unset, env_var}, socket) do
env_vars = Enum.reject(socket.assigns.env_vars, &(&1.key == env_var.key))
env_vars = Enum.reject(socket.assigns.env_vars, &(&1.name == env_var.name))
{:noreply, assign(socket, env_vars: env_vars, env_var: nil)}
end

View file

@ -7,7 +7,7 @@ defmodule Livebook.SettingsTest do
env_var = insert_env_var(:env_var)
assert env_var in Settings.fetch_env_vars()
Settings.unset_env_var(env_var.key)
Settings.unset_env_var(env_var.name)
refute env_var in Settings.fetch_env_vars()
end
@ -18,7 +18,7 @@ defmodule Livebook.SettingsTest do
Settings.fetch_env_var!("123456")
end
env_var = insert_env_var(:env_var, key: "123456")
env_var = insert_env_var(:env_var, name: "123456")
assert Settings.fetch_env_var!("123456") == env_var
Settings.unset_env_var("123456")
@ -26,7 +26,7 @@ defmodule Livebook.SettingsTest do
test "env_var_exists?/1" do
refute Settings.env_var_exists?("FOO")
insert_env_var(:env_var, key: "FOO")
insert_env_var(:env_var, name: "FOO")
assert Settings.env_var_exists?("FOO")
Settings.unset_env_var("FOO")
@ -34,13 +34,13 @@ defmodule Livebook.SettingsTest do
describe "set_env_var/1" do
test "creates an environment variable" do
attrs = params_for(:env_var, key: "FOO_BAR_BAZ")
attrs = params_for(:env_var, name: "FOO_BAR_BAZ")
assert {:ok, env_var} = Settings.set_env_var(attrs)
assert attrs.key == env_var.key
assert attrs.name == env_var.name
assert attrs.value == env_var.value
Settings.unset_env_var(env_var.key)
Settings.unset_env_var(env_var.name)
end
test "updates an environment variable" do
@ -48,19 +48,19 @@ defmodule Livebook.SettingsTest do
attrs = %{value: "FOO"}
assert {:ok, updated_env_var} = Settings.set_env_var(env_var, attrs)
assert env_var.key == updated_env_var.key
assert env_var.name == updated_env_var.name
assert updated_env_var.value == attrs.value
Settings.unset_env_var(env_var.key)
Settings.unset_env_var(env_var.name)
end
test "returns changeset error" do
attrs = params_for(:env_var, key: nil)
attrs = params_for(:env_var, name: nil)
assert {:error, changeset} = Settings.set_env_var(attrs)
assert "can't be blank" in errors_on(changeset).key
assert "can't be blank" in errors_on(changeset).name
assert {:error, changeset} = Settings.set_env_var(%{attrs | key: "LB_FOO"})
assert "cannot start with the LB_ prefix" in errors_on(changeset).key
assert {:error, changeset} = Settings.set_env_var(%{attrs | name: "LB_FOO"})
assert "cannot start with the LB_ prefix" in errors_on(changeset).name
end
end
end

View file

@ -94,9 +94,11 @@ defmodule LivebookWeb.Hub.EditLiveTest do
assert_patch(view, Routes.hub_path(conn, :add_env_var, hub.id))
assert render(view) =~ "Add environment variable"
attrs = params_for(:env_var, name: "FOO_ENV_VAR")
view
|> element("#env-var-form")
|> render_change(%{"env_var" => %{"key" => "FOO_ENV_VAR", "value" => "12345"}})
|> render_change(%{"env_var" => attrs})
refute view
|> element("#env-var-form button[disabled]")
@ -107,7 +109,7 @@ defmodule LivebookWeb.Hub.EditLiveTest do
assert {:ok, _view, html} =
view
|> element("#env-var-form")
|> render_submit(%{"env_var" => %{"key" => "FOO_ENV_VAR", "value" => "12345"}})
|> render_submit(%{"env_var" => attrs})
|> follow_redirect(conn)
assert html =~ "Environment variable added"
@ -139,9 +141,11 @@ defmodule LivebookWeb.Hub.EditLiveTest do
assert_patch(view, Routes.hub_path(conn, :edit_env_var, hub.id, "FOO_ENV_VAR"))
assert render(view) =~ "Edit environment variable"
attrs = params_for(:env_var, name: "FOO_ENV_VAR")
view
|> element("#env-var-form")
|> render_change(%{"env_var" => %{"key" => "FOO_ENV_VAR", "value" => "12345"}})
|> render_change(%{"env_var" => attrs})
refute view
|> element("#env-var-form button[disabled]")
@ -152,7 +156,7 @@ defmodule LivebookWeb.Hub.EditLiveTest do
assert {:ok, _view, html} =
view
|> element("#env-var-form")
|> render_submit(%{"env_var" => %{"key" => "FOO_ENV_VAR", "value" => "12345"}})
|> render_submit(%{"env_var" => attrs})
|> follow_redirect(conn)
assert html =~ "Environment variable updated"

View file

@ -942,51 +942,93 @@ defmodule LivebookWeb.SessionLiveTest do
end
end
test "outputs persisted env var from ets", %{conn: conn, session: session} do
Session.subscribe(session.id)
{:ok, view, _} = live(conn, "/sessions/#{session.id}")
describe "environment variables" do
test "outputs persisted env var from ets", %{conn: conn, session: session} do
Session.subscribe(session.id)
{:ok, view, _} = live(conn, "/sessions/#{session.id}")
section_id = insert_section(session.pid)
section_id = insert_section(session.pid)
cell_id =
insert_text_cell(session.pid, section_id, :code, ~s{System.get_env("MY_AWESOME_ENV")})
cell_id =
insert_text_cell(session.pid, section_id, :code, ~s{System.get_env("MY_AWESOME_ENV")})
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id, {:text, "\e[35mnil\e[0m"}, _}}
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id, {:text, "\e[35mnil\e[0m"}, _}}
attrs = params_for(:env_var, key: "MY_AWESOME_ENV", value: "MyEnvVarValue")
Settings.set_env_var(attrs)
attrs = params_for(:env_var, name: "MY_AWESOME_ENV", value: "MyEnvVarValue")
Settings.set_env_var(attrs)
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id,
{:text, "\e[32m\"MyEnvVarValue\"\e[0m"}, _}}
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id,
{:text, "\e[32m\"MyEnvVarValue\"\e[0m"}, _}}
Settings.set_env_var(%{attrs | value: "OTHER_VALUE"})
Settings.set_env_var(%{attrs | value: "OTHER_VALUE"})
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id,
{:text, "\e[32m\"OTHER_VALUE\"\e[0m"}, _}}
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id,
{:text, "\e[32m\"OTHER_VALUE\"\e[0m"}, _}}
Settings.unset_env_var("MY_AWESOME_ENV")
Settings.unset_env_var("MY_AWESOME_ENV")
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id, {:text, "\e[35mnil\e[0m"}, _}}
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id, {:text, "\e[35mnil\e[0m"}, _}}
end
@tag :tmp_dir
test "outputs persisted PATH delimited with os PATH env var",
%{conn: conn, session: session, tmp_dir: tmp_dir} do
separator =
case :os.type() do
{:win32, _} -> ";"
_ -> ":"
end
os_path = System.get_env("PATH", "")
old_expected_path = ~s/\e[32m"#{os_path}"\e[0m/
expected_path = ~s/\e[32m"#{os_path}#{separator}#{tmp_dir}"\e[0m/
attrs = params_for(:env_var, name: "PATH", value: tmp_dir)
Settings.set_env_var(attrs)
Session.subscribe(session.id)
{:ok, view, _} = live(conn, "/sessions/#{session.id}")
section_id = insert_section(session.pid)
cell_id = insert_text_cell(session.pid, section_id, :code, ~s{System.get_env("PATH")})
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id, {:text, ^expected_path}, _}}
Settings.unset_env_var("PATH")
view
|> element(~s{[data-el-session]})
|> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id})
assert_receive {:operation,
{:add_cell_evaluation_response, _, ^cell_id, {:text, ^old_expected_path}, _}}
end
end
# Helpers

View file

@ -8,19 +8,19 @@ defmodule LivebookWeb.SettingsLiveTest do
describe "environment variables configuration" do
test "list persisted environment variables", %{conn: conn} do
insert_env_var(:env_var, key: "MY_ENVIRONMENT_VAR")
insert_env_var(:env_var, name: "MY_ENVIRONMENT_VAR")
{:ok, _view, html} = live(conn, Routes.settings_path(conn, :page))
assert html =~ "MY_ENVIRONMENT_VAR"
end
test "adds an environment variable", %{conn: conn} do
attrs = params_for(:env_var, key: "JAKE_PERALTA_ENV_VAR")
attrs = params_for(:env_var, name: "JAKE_PERALTA_ENV_VAR")
{:ok, view, html} = live(conn, Routes.settings_path(conn, :add_env_var))
assert html =~ "Add environment variable"
refute html =~ attrs.key
refute html =~ attrs.name
view
|> element("#env-var-form")
@ -36,19 +36,19 @@ defmodule LivebookWeb.SettingsLiveTest do
assert_patch(view, Routes.settings_path(conn, :page))
assert render(view) =~ attrs.key
assert render(view) =~ attrs.name
end
test "updates an environment variable", %{conn: conn} do
env_var = insert_env_var(:env_var, key: "UPDATE_ME")
env_var = insert_env_var(:env_var, name: "UPDATE_ME")
{:ok, view, html} = live(conn, Routes.settings_path(conn, :page))
assert html =~ env_var.key
assert html =~ env_var.name
render_click(view, "edit_env_var", %{"env_var" => env_var.key})
render_click(view, "edit_env_var", %{"env_var" => env_var.name})
assert_patch(view, Routes.settings_path(conn, :edit_env_var, env_var.key))
assert_patch(view, Routes.settings_path(conn, :edit_env_var, env_var.name))
assert render(view) =~ "Edit environment variable"
form = element(view, "#env-var-form")
@ -63,9 +63,9 @@ defmodule LivebookWeb.SettingsLiveTest do
render_submit(form, %{"env_var" => %{"value" => "123456"}})
assert_patch(view, Routes.settings_path(conn, :page))
updated_env_var = Settings.fetch_env_var!(env_var.key)
updated_env_var = Settings.fetch_env_var!(env_var.name)
assert updated_env_var.key == env_var.key
assert updated_env_var.name == env_var.name
refute updated_env_var.value == env_var.value
end
@ -73,11 +73,11 @@ defmodule LivebookWeb.SettingsLiveTest do
env_var = insert_env_var(:env_var)
{:ok, view, html} = live(conn, Routes.settings_path(conn, :page))
assert html =~ env_var.key
assert html =~ env_var.name
render_click(view, "delete_env_var", %{"env_var" => env_var.key})
render_click(view, "delete_env_var", %{"env_var" => env_var.name})
refute render(view) =~ env_var.key
refute render(view) =~ env_var.name
end
end
end

View file

@ -33,7 +33,7 @@ defmodule Livebook.Factory do
def build(:env_var) do
%Livebook.Settings.EnvVar{
key: "BAR",
name: "BAR",
value: "foo"
}
end
@ -55,7 +55,7 @@ defmodule Livebook.Factory do
def insert_env_var(factory_name, attrs \\ %{}) do
env_var = build(factory_name, attrs)
attributes = env_var |> Map.from_struct() |> Map.to_list()
Livebook.Storage.current().insert(:env_vars, env_var.key, attributes)
Livebook.Storage.current().insert(:env_vars, env_var.name, attributes)
env_var
end