New LivebookProto messages and minor improvements (#2503)

lib/livebook/hubs/team_client.ex

@@ -396,12 +396,10 @@ defmodule Livebook.Hubs.TeamClient do
   end
 
   defp handle_event(:deployment_group_deleted, deployment_group_deleted, state) do
-    if deployment_group =
-         Enum.find(state.deployment_groups, &(&1.id == deployment_group_deleted.id)) do
+    with {:ok, deployment_group} <- fetch_deployment_group(deployment_group_deleted.id, state) do
       Teams.Broadcasts.deployment_group_deleted(deployment_group)
+
       remove_deployment_group(state, deployment_group)
-    else
-      state
     end
   end
 
@@ -423,26 +421,18 @@ defmodule Livebook.Hubs.TeamClient do
   defp handle_event(:agent_key_created, agent_key_created, state) do
     agent_key = build_agent_key(agent_key_created)
 
-    if deployment_group =
-         find_deployment_group(agent_key.deployment_group_id, state.deployment_groups) do
-      handle_event(:deployment_group_updated, put_agent_key(deployment_group, agent_key), state)
-    else
-      state
+    with {:ok, deployment_group} <- fetch_deployment_group(agent_key.deployment_group_id, state) do
+      deployment_group = put_agent_key(deployment_group, agent_key)
+      handle_event(:deployment_group_updated, deployment_group, state)
     end
   end
 
   defp handle_event(:agent_key_deleted, agent_key_deleted, state) do
     agent_key = build_agent_key(agent_key_deleted)
 
-    if deployment_group =
-         find_deployment_group(agent_key.deployment_group_id, state.deployment_groups) do
-      handle_event(
-        :deployment_group_updated,
-        remove_agent_key(deployment_group, agent_key),
-        state
-      )
-    else
-      state
+    with {:ok, deployment_group} <- fetch_deployment_group(agent_key.deployment_group_id, state) do
+      deployment_group = remove_agent_key(deployment_group, agent_key)
+      handle_event(:deployment_group_updated, deployment_group, state)
     end
   end
 
@@ -528,6 +518,14 @@ defmodule Livebook.Hubs.TeamClient do
   defp find_deployment_group(nil, _), do: nil
   defp find_deployment_group(id, groups), do: Enum.find(groups, &(&1.id == id))
 
+  defp fetch_deployment_group(id, state) do
+    if deployment_group = find_deployment_group(id, state.deployment_groups) do
+      {:ok, deployment_group}
+    else
+      state
+    end
+  end
+
   # We cannot use to_existing_atom because the atoms
   # may not have been loaded. Luckily, we can trust
   # on Livebook Teams as a source.

lib/livebook/teams/requests.ex

@@ -96,7 +96,7 @@ defmodule Livebook.Teams.Requests do
   Send a request to Livebook Team API to delete a secret.
   """
   @spec delete_secret(Team.t(), Secret.t()) ::
-          {:ok, String.t()} | {:error, map() | String.t()} | {:transport_error, String.t()}
+          {:ok, map()} | {:error, map() | String.t()} | {:transport_error, String.t()}
   def delete_secret(team, %{deployment_group_id: nil} = secret) do
     delete("/api/v1/org/secrets", %{name: secret.name}, team)
   end
@@ -156,7 +156,7 @@ defmodule Livebook.Teams.Requests do
   Send a request to Livebook Team API to delete a file system.
   """
   @spec delete_file_system(Team.t(), FileSystem.t()) ::
-          {:ok, String.t()} | {:error, map() | String.t()} | {:transport_error, String.t()}
+          {:ok, map()} | {:error, map() | String.t()} | {:transport_error, String.t()}
   def delete_file_system(team, file_system) do
     delete("/api/v1/org/file-systems", %{id: file_system.external_id}, team)
   end
@@ -210,7 +210,7 @@ defmodule Livebook.Teams.Requests do
   Send a request to Livebook Team API to delete an agent key.
   """
   @spec delete_agent_key(Team.t(), AgentKey.t()) ::
-          {:ok, String.t()} | {:error, map() | String.t()} | {:transport_error, String.t()}
+          {:ok, map()} | {:error, map() | String.t()} | {:transport_error, String.t()}
   def delete_agent_key(team, agent_key) do
     params = %{id: agent_key.id, deployment_group_id: agent_key.deployment_group_id}
     delete("/api/v1/org/deployment-groups/agent-keys", params, team)

lib/livebook_web/live/hub/teams/deployment_group_form_component.ex

@@ -37,6 +37,11 @@ defmodule LivebookWeb.Hub.Teams.DeploymentGroupFormComponent do
       <p class="text-gray-700">
         <%= @subtitle %>
       </p>
+
+      <div :if={@error_message} class="error-box">
+        <%= @error_message %>
+      </div>
+
       <div class="flex flex-columns gap-4">
         <.form
           :let={f}

proto/lib/livebook_proto.ex

@@ -3,7 +3,7 @@ defmodule LivebookProto do
     AgentConnected,
     AgentKeyCreated,
     AgentKeyDeleted,
-    Event,
+    AppDeploymentCreated,
     FileSystemCreated,
     FileSystemDeleted,
     FileSystemUpdated,
@@ -16,7 +16,7 @@ defmodule LivebookProto do
     UserConnected
   }
 
-  @event_mapping (for {_id, field_prop} <- Event.__message_props__().field_props,
+  @event_mapping (for {_id, field_prop} <- LivebookProto.Event.__message_props__().field_props,
                       into: %{} do
                     {field_prop.type, field_prop.name_atom}
                   end)
@@ -25,6 +25,7 @@ defmodule LivebookProto do
           AgentConnected.t()
           | AgentKeyCreated.t()
           | AgentKeyDeleted.t()
+          | AppDeploymentCreated.t()
           | FileSystemCreated.t()
           | FileSystemDeleted.t()
           | FileSystemUpdated.t()
@@ -39,9 +40,9 @@ defmodule LivebookProto do
   @doc """
   Builds an event with given data.
   """
-  @spec build_event(event_proto()) :: Event.t()
+  @spec build_event(event_proto()) :: LivebookProto.Event.t()
   def build_event(%struct{} = data) do
-    %Event{type: {event_type(struct), data}}
+    %LivebookProto.Event{type: {event_type(struct), data}}
   end
 
   defp event_type(module), do: Map.fetch!(@event_mapping, module)

proto/lib/livebook_proto/app_deployment_created.pb.ex

@@ -0,0 +1,13 @@
+defmodule LivebookProto.AppDeploymentCreated do
+  use Protobuf, syntax: :proto3, protoc_gen_elixir_version: "0.12.0"
+
+  field :id, 1, type: :string
+  field :title, 2, type: :string
+  field :sha, 3, type: :string
+  field :archive_url, 4, type: :string, json_name: "archiveUrl"
+  field :app_id, 5, type: :string, json_name: "appId"
+  field :slug, 6, type: :string
+  field :deployment_group_id, 7, type: :string, json_name: "deploymentGroupId"
+  field :deployed_by, 8, type: :string, json_name: "deployedBy"
+  field :deployed_at, 9, type: :string, json_name: "deployedAt"
+end

proto/lib/livebook_proto/deployed_app.pb.ex

@@ -0,0 +1,13 @@
+defmodule LivebookProto.DeployedApp do
+  use Protobuf, syntax: :proto3, protoc_gen_elixir_version: "0.12.0"
+
+  field :id, 1, type: :string
+  field :title, 2, type: :string
+  field :sha, 3, type: :string
+  field :archive_url, 4, type: :string, json_name: "archiveUrl"
+  field :app_id, 5, type: :string, json_name: "appId"
+  field :slug, 6, type: :string
+  field :deployment_group_id, 7, type: :string, json_name: "deploymentGroupId"
+  field :deployed_by, 8, type: :string, json_name: "deployedBy"
+  field :deployed_at, 9, type: :string, json_name: "deployedAt"
+end

proto/lib/livebook_proto/deployment_group.pb.ex

@@ -9,4 +9,9 @@ defmodule LivebookProto.DeploymentGroup do
   field :zta_provider, 6, type: :string, json_name: "ztaProvider"
   field :zta_key, 7, type: :string, json_name: "ztaKey"
   field :agent_keys, 8, repeated: true, type: LivebookProto.AgentKey, json_name: "agentKeys"
+
+  field :deployed_apps, 9,
+    repeated: true,
+    type: LivebookProto.DeployedApp,
+    json_name: "deployedApps"
 end

proto/lib/livebook_proto/deployment_group_created.pb.ex

@@ -9,4 +9,9 @@ defmodule LivebookProto.DeploymentGroupCreated do
   field :zta_provider, 6, type: :string, json_name: "ztaProvider"
   field :zta_key, 7, type: :string, json_name: "ztaKey"
   field :agent_keys, 8, repeated: true, type: LivebookProto.AgentKey, json_name: "agentKeys"
+
+  field :deployed_apps, 9,
+    repeated: true,
+    type: LivebookProto.DeployedApp,
+    json_name: "deployedApps"
 end

proto/lib/livebook_proto/deployment_group_updated.pb.ex

@@ -9,4 +9,9 @@ defmodule LivebookProto.DeploymentGroupUpdated do
   field :zta_provider, 6, type: :string, json_name: "ztaProvider"
   field :zta_key, 7, type: :string, json_name: "ztaKey"
   field :agent_keys, 8, repeated: true, type: LivebookProto.AgentKey, json_name: "agentKeys"
+
+  field :deployed_apps, 9,
+    repeated: true,
+    type: LivebookProto.DeployedApp,
+    json_name: "deployedApps"
 end

proto/lib/livebook_proto/event.pb.ex

@@ -67,4 +67,9 @@ defmodule LivebookProto.Event do
     type: LivebookProto.AgentKeyDeleted,
     json_name: "agentKeyDeleted",
     oneof: 0
+
+  field :app_deployment_created, 14,
+    type: LivebookProto.AppDeploymentCreated,
+    json_name: "appDeploymentCreated",
+    oneof: 0
 end

proto/messages.proto

@@ -63,6 +63,7 @@ message DeploymentGroup {
   string zta_provider = 6;
   string zta_key = 7;
   repeated AgentKey agent_keys = 8;
+  repeated DeployedApp deployed_apps = 9;
 }
 
 message DeploymentGroupCreated {
@@ -74,6 +75,7 @@ message DeploymentGroupCreated {
   string zta_provider = 6;
   string zta_key = 7;
   repeated AgentKey agent_keys = 8;
+  repeated DeployedApp deployed_apps = 9;
 }
 
 message DeploymentGroupUpdated {
@@ -85,6 +87,7 @@ message DeploymentGroupUpdated {
   string zta_provider = 6;
   string zta_key = 7;
   repeated AgentKey agent_keys = 8;
+  repeated DeployedApp deployed_apps = 9;
 }
 
 message DeploymentGroupDeleted {
@@ -126,6 +129,30 @@ message AgentConnected {
   repeated DeploymentGroup deployment_groups = 7;
 }
 
+message DeployedApp {
+  string id = 1;
+  string title = 2;
+  string sha = 3;
+  string archive_url = 4;
+  string app_id = 5;
+  string slug = 6;
+  string deployment_group_id = 7;
+  string deployed_by = 8;
+  string deployed_at = 9;
+}
+
+message AppDeploymentCreated {
+  string id = 1;
+  string title = 2;
+  string sha = 3;
+  string archive_url = 4;
+  string app_id = 5;
+  string slug = 6;
+  string deployment_group_id = 7;
+  string deployed_by = 8;
+  string deployed_at = 9;
+}
+
 message Event {
   oneof type {
     SecretCreated secret_created = 1;
@@ -141,5 +168,6 @@ message Event {
     AgentConnected agent_connected = 11;
     AgentKeyCreated agent_key_created = 12;
     AgentKeyDeleted agent_key_deleted = 13;
+    AppDeploymentCreated app_deployment_created = 14;
   }
 }

test/livebook_teams/web/hub/deployment_group_live_test.exs

@@ -116,6 +116,65 @@ defmodule LivebookWeb.Integration.Hub.DeploymentGroupLiveTest do
     assert updated_deployment_group in Livebook.Teams.get_deployment_groups(hub)
   end
 
+  test "returns error if something goes wrong", %{conn: conn, hub: hub, node: node} do
+    name = "TEAMS_ERROR_DEPLOYMENT_GROUP"
+    mode = :online
+    insert_deployment_group(name: name, mode: mode, hub_id: hub.id)
+
+    assert_receive {:deployment_group_created,
+                    %DeploymentGroup{name: ^name, mode: ^mode} = deployment_group}
+
+    attrs = %{
+      deployment_group: %{
+        id: deployment_group.id,
+        name: deployment_group.name,
+        mode: deployment_group.mode,
+        hub_id: deployment_group.hub_id
+      }
+    }
+
+    new_name = "FOO"
+
+    {:ok, view, html} =
+      live(conn, ~p"/hub/#{hub.id}/deployment-groups/edit/#{deployment_group.id}")
+
+    assert html =~ "Edit deployment group"
+
+    assert html =~
+             "Manage the #{deployment_group.name} (#{deployment_group.mode}) deployment group"
+
+    view
+    |> element("#deployment-groups-form")
+    |> render_change(attrs)
+
+    refute view
+           |> element("#deployment-groups-form button[disabled]")
+           |> has_element?()
+
+    # Forces the server to delete the deployment group
+    teams_deployment_group =
+      erpc_call(node, :get_deployment_group!, [String.to_integer(deployment_group.id)])
+
+    {:ok, _} = erpc_call(node, :delete_deployment_group, [teams_deployment_group])
+
+    view
+    |> element("#deployment-groups-form")
+    |> render_submit(put_in(attrs.deployment_group.name, new_name))
+
+    updated_deployment_group = %{deployment_group | name: new_name}
+
+    refute_receive {:deployment_group_updated, ^updated_deployment_group}
+
+    assert render(view) =~
+             "Something went wrong, try again later or please file a bug if it persists"
+
+    refute updated_deployment_group in Livebook.Teams.get_deployment_groups(hub)
+
+    # In this case, we aren't expecting the delete message,
+    # so the old deployment group remains in the list
+    assert deployment_group in Livebook.Teams.get_deployment_groups(hub)
+  end
+
   test "creates a secret", %{conn: conn, hub: hub} do
     insert_deployment_group(
       name: "TEAMS_EDIT_DEPLOYMENT_GROUP",

test/support/integration/teams_server.ex

@@ -206,14 +206,6 @@ defmodule Livebook.TeamsServer do
     System.get_env("TEAMS_PORT", "4123")
   end
 
-  defp debug do
-    System.get_env("TEAMS_DEBUG", "false")
-  end
-
-  defp proto do
-    System.get_env("TEAMS_LIVEBOOK_PROTO_PATH")
-  end
-
   defp wait_on_start(state, port) do
     url = state.url || fetch_url(state)
 
@@ -253,13 +245,16 @@ defmodule Livebook.TeamsServer do
   end
 
   defp env(app_port, state_env) do
-    env = %{
-      "MIX_ENV" => "livebook",
-      "PORT" => to_string(app_port),
-      "DEBUG" => debug()
-    }
-
-    env = if proto(), do: Map.merge(env, %{"LIVEBOOK_PROTO_PATH" => proto()}), else: env
+    env =
+      Map.filter(
+        %{
+          "MIX_ENV" => "livebook",
+          "PORT" => to_string(app_port),
+          "DEBUG" => System.get_env("TEAMS_DEBUG", "false"),
+          "LIVEBOOK_PROTO_PATH" => System.get_env("TEAMS_LIVEBOOK_PROTO_PATH")
+        },
+        fn {_key, value} -> value not in ["", nil] end
+      )
 
     if state_env do
       Map.merge(env, state_env)