Skip to content

Commit 3e7f6b5

Browse files
authored
Skip deployment from CLI for unchanged apps (#3116)
1 parent c97fc71 commit 3e7f6b5

File tree

6 files changed

+187
-64
lines changed

6 files changed

+187
-64
lines changed

lib/livebook/teams.ex

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -267,17 +267,17 @@ defmodule Livebook.Teams do
267267
@doc """
268268
Deploys the given app deployment to given deployment group using an org token.
269269
"""
270-
@spec deploy_app_from_cli(Team.t(), Teams.AppDeployment.t(), integer()) ::
271-
{:ok, String.t()} | {:error, map()} | {:transport_error, String.t()}
270+
@spec deploy_app_from_cli(Team.t(), Teams.AppDeployment.t(), integer(), keyword()) ::
271+
{:ok, map()} | {:error, map()} | {:transport_error, String.t()}
272272
def deploy_app_from_cli(
273273
%Team{} = team,
274274
%Teams.AppDeployment{} = app_deployment,
275-
deployment_group_id
275+
deployment_group_id,
276+
opts \\ []
276277
) do
277-
case Requests.deploy_app_from_cli(team, app_deployment, deployment_group_id) do
278-
{:ok, %{"url" => url}} -> {:ok, url}
279-
{:error, %{"errors" => errors}} -> {:error, errors}
280-
any -> any
278+
with {:error, %{"errors" => errors}} <-
279+
Requests.deploy_app_from_cli(team, app_deployment, deployment_group_id, opts) do
280+
{:error, errors}
281281
end
282282
end
283283

lib/livebook/teams/app_deployment.ex

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,20 @@ defmodule Livebook.Teams.AppDeployment do
6363
end
6464
end
6565

66-
def new(%Livebook.Notebook{} = notebook, source, files_dir) do
66+
defp new(%Livebook.Notebook{} = notebook, source, files_dir) do
6767
with {:ok, files} <- build_and_check_file_entries(notebook, source, files_dir),
6868
{:ok, {_, zip_content}} <- :zip.create(~c"app_deployment.zip", files, [:memory]),
6969
:ok <- validate_size(zip_content) do
70-
md5_hash = :crypto.hash(:md5, zip_content)
70+
md5_hash =
71+
:crypto.hash(
72+
:md5,
73+
files
74+
|> Enum.sort()
75+
|> Enum.map(fn {file, contents} ->
76+
["__FILE__", file, contents]
77+
end)
78+
)
79+
7180
shasum = Base.encode16(md5_hash, case: :lower)
7281

7382
{:ok,

lib/livebook/teams/requests.ex

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -241,19 +241,22 @@ defmodule Livebook.Teams.Requests do
241241
@doc """
242242
Send a request to Livebook Team API to deploy an app using an org token.
243243
"""
244-
@spec deploy_app_from_cli(Team.t(), Teams.AppDeployment.t(), integer()) :: api_result()
245-
def deploy_app_from_cli(team, app_deployment, deployment_group_id) do
244+
@spec deploy_app_from_cli(Team.t(), Teams.AppDeployment.t(), integer(), keyword()) ::
245+
api_result()
246+
def deploy_app_from_cli(team, app_deployment, deployment_group_id, opts) do
246247
secret_key = Teams.derive_key(team.teams_key)
247248

248-
params = %{
249-
title: app_deployment.title,
250-
slug: app_deployment.slug,
251-
multi_session: app_deployment.multi_session,
252-
access_type: app_deployment.access_type,
253-
app_folder_id: app_deployment.app_folder_id,
254-
deployment_group_id: deployment_group_id,
255-
sha: app_deployment.sha
256-
}
249+
params =
250+
%{
251+
title: app_deployment.title,
252+
slug: app_deployment.slug,
253+
multi_session: app_deployment.multi_session,
254+
access_type: app_deployment.access_type,
255+
app_folder_id: app_deployment.app_folder_id,
256+
deployment_group_id: deployment_group_id,
257+
sha: app_deployment.sha,
258+
redeploy: opts[:redeploy] || false
259+
}
257260

258261
encrypted_content = Teams.encrypt(app_deployment.file, secret_key)
259262
upload("/api/v1/cli/org/apps", encrypted_content, params, team)

lib/livebook_cli/deploy.ex

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ defmodule LivebookCLI.Deploy do
3737
org_token: :string,
3838
teams_key: :string,
3939
deployment_group_id: :integer,
40-
dry_run: :boolean
40+
dry_run: :boolean,
41+
redeploy: :boolean
4142
]
4243

4344
@impl true
@@ -47,6 +48,10 @@ defmodule LivebookCLI.Deploy do
4748
config = config_from_args(args)
4849
ensure_config!(config)
4950

51+
if config.redeploy? do
52+
log_warning("Note: --redeploy flag set, all apps will be deployed regardless of changes.")
53+
end
54+
5055
team = authenticate_cli!(config)
5156
deploy_to_teams(team, config)
5257
end
@@ -59,7 +64,8 @@ defmodule LivebookCLI.Deploy do
5964
session_token: opts[:org_token],
6065
teams_key: opts[:teams_key],
6166
deployment_group_id: opts[:deployment_group_id],
62-
dry_run?: opts[:dry_run] || false
67+
dry_run?: opts[:dry_run] || false,
68+
redeploy?: opts[:redeploy] || false
6369
}
6470
end
6571

@@ -160,10 +166,15 @@ defmodule LivebookCLI.Deploy do
160166
case Livebook.Teams.deploy_app_from_cli(
161167
team,
162168
app_deployment,
163-
config.deployment_group_id
169+
config.deployment_group_id,
170+
redeploy: config.redeploy?
164171
) do
165-
{:ok, url} ->
166-
log_info([:green, " * #{app_deployment.title} deployed successfully. (#{url})"])
172+
{:ok, %{"url" => url, "state" => "deployed"}} ->
173+
url = if url, do: " (#{url})", else: ""
174+
log_info([:green, " * #{app_deployment.title} deployed successfully.", url])
175+
176+
{:ok, %{"state" => "unchanged"}} ->
177+
log_info([:blue, " * #{app_deployment.title} unchanged, skipping"])
167178

168179
{:error, errors} ->
169180
log_error(" * #{app_deployment.title} failed to deploy.")

test/livebook_teams/cli/deploy_test.exs

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,46 @@ defmodule LivebookCLI.Integration.DeployTest do
6262
app_folder_id: ^app_folder_id,
6363
hub_id: ^hub_id,
6464
deployed_by: "CLI"
65-
}}
65+
} = app_deployment}
66+
67+
assert_receive {:app_deployment_started, ^app_deployment}
68+
69+
# skip if app had no changes
70+
output =
71+
ExUnit.CaptureIO.capture_io(fn ->
72+
assert deploy(
73+
key,
74+
team.teams_key,
75+
deployment_group.id,
76+
app_path
77+
) == :ok
78+
end)
79+
80+
assert output =~ "* Preparing to deploy notebook #{slug}.livemd"
81+
assert output =~ " * #{title} unchanged, skipping"
82+
83+
refute_receive {:app_deployment_started, ^app_deployment}
84+
85+
# should not show `(url/apps/slug)` in the app deployment output
86+
{:ok, deployment_group} =
87+
TeamsRPC.update_deployment_group(node, deployment_group, %{url: nil})
88+
89+
# force app to redeploy
90+
output =
91+
ExUnit.CaptureIO.capture_io(fn ->
92+
assert deploy(
93+
key,
94+
team.teams_key,
95+
deployment_group.id,
96+
app_path,
97+
["--redeploy"]
98+
) == :ok
99+
end)
100+
101+
assert output =~ "* Preparing to deploy notebook #{slug}.livemd"
102+
assert output =~ " * #{title} deployed successfully."
103+
104+
assert_receive {:app_deployment_started, ^app_deployment}
66105
end
67106

68107
test "successfully deploys multiple notebooks from directory",
@@ -482,12 +521,8 @@ defmodule LivebookCLI.Integration.DeployTest do
482521
end)
483522
end
484523

485-
test "successfully runs without deploying when dry run requested", %{
486-
team: team,
487-
node: node,
488-
org: org,
489-
tmp_dir: tmp_dir
490-
} do
524+
test "successfully runs without deploying when dry run requested",
525+
%{team: team, node: node, org: org, tmp_dir: tmp_dir} do
491526
title = "Test CLI Deploy App"
492527
slug = Utils.random_short_id()
493528
app_path = Path.join(tmp_dir, "#{slug}.livemd")

test/livebook_teams/teams_test.exs

Lines changed: 97 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -298,50 +298,49 @@ defmodule Livebook.TeamsTest do
298298
end
299299
end
300300

301-
describe "deploy_app_from_cli/2" do
301+
describe "deploy_app_from_cli/4" do
302302
@describetag teams_for: :user
303+
@describetag :tmp_dir
303304

304-
@tag :tmp_dir
305-
test "deploys app to Teams using a CLI session",
306-
%{team: team, node: node, tmp_dir: tmp_dir, org: org} do
307-
%{id: id} =
308-
TeamsRPC.create_deployment_group(node,
309-
name: "angry-cat-#{Ecto.UUID.generate()}",
310-
url: "http://localhost:4123",
311-
mode: :online,
312-
org: org
313-
)
314-
315-
id = to_string(id)
316-
hub_id = "team-#{org.name}"
317-
slug = Utils.random_short_id()
318-
title = "MyNotebook-#{slug}"
319-
app_settings = %{Notebook.AppSettings.new() | slug: slug}
305+
setup %{team: team, test: test, node: node, org: org, tmp_dir: tmp_dir} do
306+
deployment_group = TeamsRPC.create_deployment_group(node, mode: :online, org: org)
307+
id = to_string(deployment_group.id)
308+
assert_receive {:deployment_group_created, %{id: ^id}}
320309

321310
notebook = %{
322311
Notebook.new()
323-
| app_settings: app_settings,
324-
name: title,
325-
hub_id: hub_id,
326-
deployment_group_id: id
312+
| app_settings: %{Notebook.AppSettings.new() | slug: Utils.random_short_id()},
313+
name: to_string(test),
314+
hub_id: "team-#{org.name}",
315+
deployment_group_id: to_string(deployment_group.id)
327316
}
328317

329318
files_dir = FileSystem.File.local(tmp_dir)
330319

331-
# stamp the notebook
332-
assert {:ok, app_deployment} = Teams.AppDeployment.new(notebook, files_dir)
320+
# export to livemd to stamp the notebook (must be done before CLI session)
321+
{source, []} = Livebook.LiveMarkdown.notebook_to_livemd(notebook)
322+
323+
# create the app deployment from stamped source
324+
assert {:ok, app_deployment} = Teams.AppDeployment.new(source, files_dir)
333325

334326
# fetch the cli session
335327
{key, _org_token} = TeamsRPC.create_org_token(node, org: org)
336328
config = %{teams_key: team.teams_key, session_token: key}
337329
assert {:ok, team} = Teams.fetch_cli_session(config)
338330

339-
# deploy the app
340-
assert {:ok, _url} = Teams.deploy_app_from_cli(team, app_deployment, id)
331+
%{app_deployment: app_deployment, team: team, source: source, files_dir: files_dir}
332+
end
333+
334+
test "deploys app to Teams using a CLI session",
335+
%{app_deployment: app_deployment, team: team} = ctx do
336+
id = app_deployment.deployment_group_id
337+
assert {:ok, %{"state" => "deployed"}} = Teams.deploy_app_from_cli(team, app_deployment, id)
341338

339+
slug = app_deployment.slug
340+
title = app_deployment.title
342341
sha = app_deployment.sha
343-
multi_session = app_settings.multi_session
344-
access_type = app_settings.access_type
342+
multi_session = app_deployment.multi_session
343+
access_type = app_deployment.access_type
345344

346345
assert_receive {:app_deployment_started,
347346
%Livebook.Teams.AppDeployment{
@@ -352,14 +351,60 @@ defmodule Livebook.TeamsTest do
352351
multi_session: ^multi_session,
353352
access_type: ^access_type,
354353
deployment_group_id: ^id
355-
} = app_deployment2}
354+
} = app_deployment_from_ws}
355+
356+
# force app deployment to be stopped
357+
TeamsRPC.toggle_app_deployment(ctx.node, app_deployment_from_ws.id, team.org_id)
358+
assert_receive {:app_deployment_stopped, ^app_deployment_from_ws}
359+
end
360+
361+
test "skips if the is still the same version, unless forced to redeploy",
362+
%{app_deployment: app_deployment, team: team} = ctx do
363+
id = app_deployment.deployment_group_id
364+
assert {:ok, %{"state" => "deployed"}} = Teams.deploy_app_from_cli(team, app_deployment, id)
365+
366+
app_deployment_from_ws = assert_app_deployment_started(app_deployment)
367+
368+
# must skip
369+
{:ok, regenerated_app_deployment} = Teams.AppDeployment.new(ctx.source, ctx.files_dir)
370+
assert app_deployment.sha == regenerated_app_deployment.sha
371+
372+
assert {:ok, %{"state" => "unchanged"}} =
373+
Teams.deploy_app_from_cli(team, regenerated_app_deployment, id)
374+
375+
refute_receive {:app_deployment_started, ^app_deployment_from_ws}
376+
377+
# must redeploy
378+
{:ok, app_deployment} = Teams.AppDeployment.new(ctx.source, ctx.files_dir)
379+
380+
assert {:ok, %{"state" => "deployed"}} =
381+
Teams.deploy_app_from_cli(team, app_deployment, id, redeploy: true)
382+
383+
app_deployment_from_ws = assert_app_deployment_started(app_deployment)
384+
385+
# force app deployment to be stopped
386+
TeamsRPC.toggle_app_deployment(ctx.node, app_deployment_from_ws.id, team.org_id)
387+
assert_receive {:app_deployment_stopped, ^app_deployment_from_ws}
388+
389+
# must redeploy because now the app is inactive
390+
{:ok, app_deployment} = Teams.AppDeployment.new(ctx.source, ctx.files_dir)
391+
assert {:ok, %{"state" => "deployed"}} = Teams.deploy_app_from_cli(team, app_deployment, id)
392+
393+
assert_app_deployment_started(app_deployment)
394+
end
395+
396+
test "returns error when has invalid data", %{app_deployment: app_deployment, team: team} do
397+
id = app_deployment.deployment_group_id
356398

357399
assert Teams.deploy_app_from_cli(team, app_deployment, 999) ==
358400
{:error, %{"deployment_group_id" => ["does not exist"]}}
359401

360402
assert Teams.deploy_app_from_cli(team, %{app_deployment | slug: "@abc"}, id) ==
361403
{:error, %{"slug" => ["should only contain alphanumeric characters and dashes"]}}
362404

405+
assert Teams.deploy_app_from_cli(team, %{app_deployment | title: nil}, id) ==
406+
{:error, %{"title" => ["can't be blank"]}}
407+
363408
assert Teams.deploy_app_from_cli(team, %{app_deployment | multi_session: nil}, id) ==
364409
{:error, %{"multi_session" => ["can't be blank"]}}
365410

@@ -368,10 +413,30 @@ defmodule Livebook.TeamsTest do
368413

369414
assert Teams.deploy_app_from_cli(team, %{app_deployment | access_type: :abc}, id) ==
370415
{:error, %{"access_type" => ["is invalid"]}}
371-
372-
# force app deployment to be stopped
373-
TeamsRPC.toggle_app_deployment(node, app_deployment2.id, team.org_id)
374-
assert_receive {:app_deployment_stopped, ^app_deployment2}
375416
end
376417
end
418+
419+
defp assert_app_deployment_started(app_deployment) do
420+
id = app_deployment.deployment_group_id
421+
slug = app_deployment.slug
422+
title = app_deployment.title
423+
sha = app_deployment.sha
424+
multi_session = app_deployment.multi_session
425+
access_type = app_deployment.access_type
426+
427+
assert_receive {:app_deployment_started,
428+
%Livebook.Teams.AppDeployment{
429+
slug: ^slug,
430+
sha: ^sha,
431+
title: ^title,
432+
deployed_by: "CLI",
433+
multi_session: ^multi_session,
434+
access_type: ^access_type,
435+
deployment_group_id: ^id
436+
} = app_deployment_from_ws}
437+
438+
assert_receive {:app_deployment_started, ^app_deployment_from_ws}
439+
440+
app_deployment_from_ws
441+
end
377442
end

0 commit comments

Comments
 (0)