From 5e7f8a477a9e3154aac8c403cd237d9c73687bd1 Mon Sep 17 00:00:00 2001 From: Alexandre de Souza Date: Tue, 16 Apr 2024 12:11:15 -0300 Subject: [PATCH] Get App Settings fields from WebSocket event (#2571) --- lib/livebook/hubs/team_client.ex | 2 ++ lib/livebook/notebook/app_settings.ex | 10 +++++++++- lib/livebook/teams.ex | 12 +++--------- lib/livebook/teams/app_deployment.ex | 8 ++++++++ lib/livebook/teams/requests.ex | 9 ++++----- .../live/session_live/app_teams_component.ex | 13 ++++++------- proto/lib/livebook_proto/app_deployment.pb.ex | 2 ++ proto/messages.proto | 2 ++ test/livebook_teams/hubs/team_client_test.exs | 14 +++++++++++--- test/livebook_teams/teams/connection_test.exs | 4 ++-- test/livebook_teams/teams_test.exs | 16 ++++++++-------- .../web/hub/deployment_group_test.exs | 2 +- test/support/factory.ex | 2 ++ 13 files changed, 60 insertions(+), 36 deletions(-) diff --git a/lib/livebook/hubs/team_client.ex b/lib/livebook/hubs/team_client.ex index c690ae146..4fe167fac 100644 --- a/lib/livebook/hubs/team_client.ex +++ b/lib/livebook/hubs/team_client.ex @@ -417,6 +417,8 @@ defmodule Livebook.Hubs.TeamClient do slug: app_deployment.slug, sha: app_deployment.sha, title: app_deployment.title, + multi_session: app_deployment.multi_session, + access_type: String.to_atom(app_deployment.access_type), hub_id: state.hub.id, deployment_group_id: app_deployment.deployment_group_id, file: nil, diff --git a/lib/livebook/notebook/app_settings.ex b/lib/livebook/notebook/app_settings.ex index 7b6d4a7e9..d48bb769f 100644 --- a/lib/livebook/notebook/app_settings.ex +++ b/lib/livebook/notebook/app_settings.ex @@ -20,6 +20,8 @@ defmodule Livebook.Notebook.AppSettings do @type access_type :: :public | :protected @type output_type :: :all | :rich + @access_types [:public, :protected] + @primary_key false embedded_schema do field :slug, :string @@ -27,7 +29,7 @@ defmodule Livebook.Notebook.AppSettings do field :zero_downtime, :boolean field :show_existing_sessions, :boolean field :auto_shutdown_ms, :integer - field :access_type, Ecto.Enum, values: [:public, :protected] + field :access_type, Ecto.Enum, values: @access_types field :password, :string field :show_source, :boolean field :output_type, Ecto.Enum, values: [:all, :rich] @@ -153,4 +155,10 @@ defmodule Livebook.Notebook.AppSettings do def valid?(settings) do change(settings).valid? end + + @doc """ + Returns a list of possible access types. + """ + @spec access_types() :: list(atom()) + def access_types(), do: @access_types end diff --git a/lib/livebook/teams.ex b/lib/livebook/teams.ex index 320957de9..3f7006072 100644 --- a/lib/livebook/teams.ex +++ b/lib/livebook/teams.ex @@ -4,7 +4,6 @@ defmodule Livebook.Teams do alias Livebook.Hubs alias Livebook.Hubs.Team alias Livebook.Hubs.TeamClient - alias Livebook.Notebook.AppSettings alias Livebook.Teams.{Agent, AppDeployment, DeploymentGroup, Org, Requests} import Ecto.Changeset, @@ -191,12 +190,12 @@ defmodule Livebook.Teams do @doc """ Creates a new app deployment. """ - @spec deploy_app(Team.t(), AppDeployment.t(), AppSettings.t()) :: + @spec deploy_app(Team.t(), AppDeployment.t()) :: :ok | {:error, Ecto.Changeset.t()} | {:transport_error, String.t()} - def deploy_app(%Team{} = team, %AppDeployment{} = app_deployment, %AppSettings{} = app_settings) do - case Requests.deploy_app(team, app_deployment, app_settings) do + def deploy_app(%Team{} = team, %AppDeployment{} = app_deployment) do + case Requests.deploy_app(team, app_deployment) do {:ok, %{"id" => _id}} -> :ok @@ -204,11 +203,6 @@ defmodule Livebook.Teams do {:error, Requests.add_errors(app_deployment, %{"file" => [error]})} {:error, %{"errors" => errors}} -> - errors = - errors - |> map_teams_field_to_livebook_field("multi_session", "file") - |> map_teams_field_to_livebook_field("access_type", "file") - {:error, Requests.add_errors(app_deployment, errors)} any -> diff --git a/lib/livebook/teams/app_deployment.ex b/lib/livebook/teams/app_deployment.ex index 748dff8dd..47ec1c8cc 100644 --- a/lib/livebook/teams/app_deployment.ex +++ b/lib/livebook/teams/app_deployment.ex @@ -9,6 +9,8 @@ defmodule Livebook.Teams.AppDeployment do slug: String.t() | nil, sha: String.t() | nil, title: String.t() | nil, + multi_session: boolean(), + access_type: Livebook.Notebook.AppSettings.access_type(), hub_id: String.t() | nil, deployment_group_id: String.t() | nil, file: binary() | nil, @@ -16,11 +18,15 @@ defmodule Livebook.Teams.AppDeployment do deployed_at: NaiveDateTime.t() | nil } + @access_types Livebook.Notebook.AppSettings.access_types() + @primary_key {:id, :string, autogenerate: false} embedded_schema do field :slug, :string field :sha, :string field :title, :string + field :multi_session, :boolean + field :access_type, Ecto.Enum, values: @access_types field :hub_id, :string field :deployment_group_id, :string field :file, :string @@ -47,6 +53,8 @@ defmodule Livebook.Teams.AppDeployment do slug: notebook.app_settings.slug, sha: shasum, title: notebook.name, + multi_session: notebook.app_settings.multi_session, + access_type: notebook.app_settings.access_type, hub_id: notebook.hub_id, deployment_group_id: notebook.deployment_group_id, file: zip_content diff --git a/lib/livebook/teams/requests.ex b/lib/livebook/teams/requests.ex index d2e12b937..77ad2d073 100644 --- a/lib/livebook/teams/requests.ex +++ b/lib/livebook/teams/requests.ex @@ -2,7 +2,6 @@ defmodule Livebook.Teams.Requests do alias Livebook.FileSystem alias Livebook.FileSystems alias Livebook.Hubs.Team - alias Livebook.Notebook.AppSettings alias Livebook.Secrets.Secret alias Livebook.Teams alias Livebook.Teams.{AppDeployment, DeploymentGroup, Org} @@ -184,16 +183,16 @@ defmodule Livebook.Teams.Requests do @doc """ Send a request to Livebook Team API to deploy an app. """ - @spec deploy_app(Team.t(), AppDeployment.t(), AppSettings.t()) :: + @spec deploy_app(Team.t(), AppDeployment.t()) :: {:ok, map()} | {:error, map() | String.t()} | {:transport_error, String.t()} - def deploy_app(team, app_deployment, app_settings) do + def deploy_app(team, app_deployment) do secret_key = Teams.derive_key(team.teams_key) params = %{ title: app_deployment.title, slug: app_deployment.slug, - multi_session: app_settings.multi_session, - access_type: app_settings.access_type, + multi_session: app_deployment.multi_session, + access_type: app_deployment.access_type, deployment_group_id: app_deployment.deployment_group_id, sha: app_deployment.sha } diff --git a/lib/livebook_web/live/session_live/app_teams_component.ex b/lib/livebook_web/live/session_live/app_teams_component.ex index a86fe7b81..cb2fc0e94 100644 --- a/lib/livebook_web/live/session_live/app_teams_component.ex +++ b/lib/livebook_web/live/session_live/app_teams_component.ex @@ -165,10 +165,8 @@ defmodule LivebookWeb.SessionLive.AppTeamsComponent do end def handle_event("deploy_app", _, socket) do - notebook = Livebook.Session.get_notebook(socket.assigns.session.pid) - - with {:ok, app_deployment} <- pack_app(socket, notebook), - :ok <- deploy_app(socket, app_deployment, notebook.app_settings) do + with {:ok, app_deployment} <- pack_app(socket), + :ok <- deploy_app(socket, app_deployment) do message = "App deployment for #{app_deployment.slug} with title #{app_deployment.title} created successfully" @@ -176,7 +174,8 @@ defmodule LivebookWeb.SessionLive.AppTeamsComponent do end end - defp pack_app(socket, notebook) do + defp pack_app(socket) do + notebook = Livebook.Session.get_notebook(socket.assigns.session.pid) files_dir = socket.assigns.session.files_dir case Livebook.Teams.AppDeployment.new(notebook, files_dir) do @@ -193,8 +192,8 @@ defmodule LivebookWeb.SessionLive.AppTeamsComponent do end end - defp deploy_app(socket, app_deployment, app_settings) do - case Livebook.Teams.deploy_app(socket.assigns.hub, app_deployment, app_settings) do + defp deploy_app(socket, app_deployment) do + case Livebook.Teams.deploy_app(socket.assigns.hub, app_deployment) do :ok -> :ok diff --git a/proto/lib/livebook_proto/app_deployment.pb.ex b/proto/lib/livebook_proto/app_deployment.pb.ex index b0ef14c83..0a13d95bf 100644 --- a/proto/lib/livebook_proto/app_deployment.pb.ex +++ b/proto/lib/livebook_proto/app_deployment.pb.ex @@ -9,4 +9,6 @@ defmodule LivebookProto.AppDeployment do field :deployment_group_id, 6, type: :string, json_name: "deploymentGroupId" field :deployed_by, 7, type: :string, json_name: "deployedBy" field :deployed_at, 8, type: :int64, json_name: "deployedAt" + field :multi_session, 9, type: :bool, json_name: "multiSession" + field :access_type, 10, type: :string, json_name: "accessType" end diff --git a/proto/messages.proto b/proto/messages.proto index 20c4e934a..8b92af2ce 100644 --- a/proto/messages.proto +++ b/proto/messages.proto @@ -124,6 +124,8 @@ message AppDeployment { string deployment_group_id = 6; string deployed_by = 7; int64 deployed_at = 8; + bool multi_session = 9; + string access_type = 10; } message AppDeploymentStarted { diff --git a/test/livebook_teams/hubs/team_client_test.exs b/test/livebook_teams/hubs/team_client_test.exs index 7baac3c74..cd7b95dfa 100644 --- a/test/livebook_teams/hubs/team_client_test.exs +++ b/test/livebook_teams/hubs/team_client_test.exs @@ -133,15 +133,19 @@ defmodule Livebook.Hubs.TeamClientTest do files_dir = Livebook.FileSystem.File.local(tmp_dir) {:ok, app_deployment} = Livebook.Teams.AppDeployment.new(notebook, files_dir) - :ok = Livebook.Teams.deploy_app(team, app_deployment, app_settings) + :ok = Livebook.Teams.deploy_app(team, app_deployment) sha = app_deployment.sha + multi_session = app_settings.multi_session + access_type = app_settings.access_type assert_receive {:app_deployment_started, %Livebook.Teams.AppDeployment{ slug: ^slug, sha: ^sha, title: ^title, + multi_session: ^multi_session, + access_type: ^access_type, deployment_group_id: ^id } = app_deployment} @@ -357,7 +361,9 @@ defmodule Livebook.Hubs.TeamClientTest do deployed_by: app_deployment.deployed_by, deployed_at: seconds, revision_id: "1", - deployment_group_id: app_deployment.deployment_group_id + deployment_group_id: app_deployment.deployment_group_id, + multi_session: app_deployment.multi_session, + access_type: to_string(app_deployment.access_type) } user_connected = %{ @@ -770,7 +776,9 @@ defmodule Livebook.Hubs.TeamClientTest do deployed_by: app_deployment.deployed_by, deployed_at: seconds, revision_id: to_string(teams_app_deployment.app_revision.id), - deployment_group_id: app_deployment.deployment_group_id + deployment_group_id: app_deployment.deployment_group_id, + multi_session: app_deployment.multi_session, + access_type: to_string(app_deployment.access_type) } agent_connected = %{agent_connected | app_deployments: [livebook_proto_app_deployment]} diff --git a/test/livebook_teams/teams/connection_test.exs b/test/livebook_teams/teams/connection_test.exs index 7b2a96c62..a0d80d8eb 100644 --- a/test/livebook_teams/teams/connection_test.exs +++ b/test/livebook_teams/teams/connection_test.exs @@ -136,7 +136,7 @@ defmodule Livebook.Teams.ConnectionTest do # since we want to fetch the app deployment from connection event, # we need to persist it before we connect to the WebSocket - :ok = Livebook.Teams.deploy_app(hub, app_deployment, app_settings) + :ok = Livebook.Teams.deploy_app(hub, app_deployment) assert {:ok, _conn} = Connection.start_link(self(), headers) assert_receive :connected @@ -181,7 +181,7 @@ defmodule Livebook.Teams.ConnectionTest do # since we want to fetch the app deployment from connection event, # we need to persist it before we connect to the WebSocket - :ok = Livebook.Teams.deploy_app(hub, app_deployment, app_settings) + :ok = Livebook.Teams.deploy_app(hub, app_deployment) # As we need to be Agent to receive the app deployments list to be deployed, # we will create another connection here diff --git a/test/livebook_teams/teams_test.exs b/test/livebook_teams/teams_test.exs index 7b03aac4f..a66b84513 100644 --- a/test/livebook_teams/teams_test.exs +++ b/test/livebook_teams/teams_test.exs @@ -209,21 +209,21 @@ defmodule Livebook.TeamsTest do files_dir = Livebook.FileSystem.File.local(tmp_dir) assert {:ok, app_deployment} = Teams.AppDeployment.new(notebook, files_dir) - assert Teams.deploy_app(team, app_deployment, app_settings) == :ok + assert Teams.deploy_app(team, app_deployment) == :ok assert {:error, %{errors: [slug: {"should only contain alphanumeric characters and dashes", []}]}} = - Teams.deploy_app(team, %{app_deployment | slug: "@abc"}, app_settings) + Teams.deploy_app(team, %{app_deployment | slug: "@abc"}) # Since the fields below belongs to AppSettings, we're mapping the errors to `:file` field. - assert {:error, %{errors: [file: {"can't be blank", []}]}} = - Teams.deploy_app(team, app_deployment, %{app_settings | multi_session: nil}) + assert {:error, %{errors: [multi_session: {"can't be blank", []}]}} = + Teams.deploy_app(team, %{app_deployment | multi_session: nil}) - assert {:error, %{errors: [file: {"can't be blank", []}]}} = - Teams.deploy_app(team, app_deployment, %{app_settings | access_type: nil}) + assert {:error, %{errors: [access_type: {"can't be blank", []}]}} = + Teams.deploy_app(team, %{app_deployment | access_type: nil}) - assert {:error, %{errors: [file: {"is invalid", []}]}} = - Teams.deploy_app(team, app_deployment, %{app_settings | access_type: :abc}) + assert {:error, %{errors: [access_type: {"is invalid", []}]}} = + Teams.deploy_app(team, %{app_deployment | access_type: :abc}) end end end diff --git a/test/livebook_teams/web/hub/deployment_group_test.exs b/test/livebook_teams/web/hub/deployment_group_test.exs index 22f7ffae4..6b0968dc5 100644 --- a/test/livebook_teams/web/hub/deployment_group_test.exs +++ b/test/livebook_teams/web/hub/deployment_group_test.exs @@ -280,7 +280,7 @@ defmodule LivebookWeb.Integration.Hub.DeploymentGroupTest do files_dir = Livebook.FileSystem.File.local(tmp_dir) {:ok, app_deployment} = Livebook.Teams.AppDeployment.new(notebook, files_dir) - :ok = Livebook.Teams.deploy_app(hub, app_deployment, app_settings) + :ok = Livebook.Teams.deploy_app(hub, app_deployment) assert_receive {:app_deployment_started, _} diff --git a/test/support/factory.ex b/test/support/factory.ex index 348952eb6..1f095e108 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -112,6 +112,8 @@ defmodule Livebook.Factory do sha: shasum, slug: slug, file: content, + multi_session: false, + access_type: :protected, hub_id: Livebook.Hubs.Personal.id(), deployment_group_id: "1", deployed_by: "Ada Lovelace",