From 6ead29182d4b8905a56971e5c2db9ace801a79d4 Mon Sep 17 00:00:00 2001 From: Kieran Date: Mon, 27 Jan 2025 11:33:38 -0800 Subject: [PATCH] [Enhancement] Auto-update yt-dlp (#589) * Added a command for updating yt-dlp * Added a yt-dlp update worker to run daily * Added a new file that runs post-boot when the app is ready to serve requests; put yt-dlp updater in there * Updated config to expose the current env globally; updated startup tasks to not run in test env * Removes unneeded test code --- config/config.exs | 12 +---- config/runtime.exs | 15 ++++++ lib/pinchflat/application.ex | 20 ++++---- lib/pinchflat/boot/post_boot_startup_tasks.ex | 46 +++++++++++++++++++ lib/pinchflat/boot/post_job_startup_tasks.ex | 11 ++++- lib/pinchflat/boot/pre_job_startup_tasks.ex | 9 +++- .../profiles/media_profile_deletion_worker.ex | 2 +- lib/pinchflat/yt_dlp/command_runner.ex | 18 ++++++++ lib/pinchflat/yt_dlp/update_worker.ex | 44 ++++++++++++++++++ lib/pinchflat/yt_dlp/yt_dlp_command_runner.ex | 1 + lib/pinchflat_web/endpoint.ex | 2 +- .../boot/post_boot_startup_tasks_test.exs | 16 +++++++ test/pinchflat/yt_dlp/command_runner_test.exs | 8 ++++ test/pinchflat/yt_dlp/update_worker_test.exs | 24 ++++++++++ 14 files changed, 202 insertions(+), 26 deletions(-) create mode 100644 lib/pinchflat/boot/post_boot_startup_tasks.ex create mode 100644 lib/pinchflat/yt_dlp/update_worker.ex create mode 100644 test/pinchflat/boot/post_boot_startup_tasks_test.exs create mode 100644 test/pinchflat/yt_dlp/update_worker_test.exs diff --git a/config/config.exs b/config/config.exs index 7b8d29a..f57e0cc 100644 --- a/config/config.exs +++ b/config/config.exs @@ -10,6 +10,7 @@ import Config config :pinchflat, ecto_repos: [Pinchflat.Repo], generators: [timestamp_type: :utc_datetime], + env: config_env(), # Specifying backend data here makes mocking and local testing SUPER easy yt_dlp_executable: System.find_executable("yt-dlp"), apprise_executable: System.find_executable("apprise"), @@ -49,16 +50,7 @@ config :pinchflat, PinchflatWeb.Endpoint, config :pinchflat, Oban, engine: Oban.Engines.Lite, - repo: Pinchflat.Repo, - # Keep old jobs for 30 days for display in the UI - plugins: [ - {Oban.Plugins.Pruner, max_age: 30 * 24 * 60 * 60}, - {Oban.Plugins.Cron, - crontab: [ - {"0 1 * * *", Pinchflat.Downloading.MediaRetentionWorker}, - {"0 2 * * *", Pinchflat.Downloading.MediaQualityUpgradeWorker} - ]} - ] + repo: Pinchflat.Repo # Configures the mailer # diff --git a/config/runtime.exs b/config/runtime.exs index 4f0a5ca..5624bfe 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -43,6 +43,11 @@ config :pinchflat, Pinchflat.Repo, # Some users may want to increase the number of workers that use yt-dlp to improve speeds # Others may want to decrease the number of these workers to lessen the chance of an IP ban {yt_dlp_worker_count, _} = Integer.parse(System.get_env("YT_DLP_WORKER_CONCURRENCY", "2")) +# Used to set the cron for the yt-dlp update worker. The reason for this is +# to avoid all instances of PF updating yt-dlp at the same time, which 1) +# could result in rate limiting and 2) gives me time to react if an update +# breaks something +%{hour: current_hour, minute: current_minute} = DateTime.utc_now() config :pinchflat, Oban, queues: [ @@ -52,6 +57,16 @@ config :pinchflat, Oban, media_fetching: yt_dlp_worker_count, remote_metadata: yt_dlp_worker_count, local_data: 8 + ], + plugins: [ + # Keep old jobs for 30 days for display in the UI + {Oban.Plugins.Pruner, max_age: 30 * 24 * 60 * 60}, + {Oban.Plugins.Cron, + crontab: [ + {"#{current_minute} #{current_hour} * * *", Pinchflat.YtDlp.UpdateWorker}, + {"0 1 * * *", Pinchflat.Downloading.MediaRetentionWorker}, + {"0 2 * * *", Pinchflat.Downloading.MediaQualityUpgradeWorker} + ]} ] if config_env() == :prod do diff --git a/lib/pinchflat/application.ex b/lib/pinchflat/application.ex index 969f2cb..3f823f4 100644 --- a/lib/pinchflat/application.ex +++ b/lib/pinchflat/application.ex @@ -9,8 +9,12 @@ defmodule Pinchflat.Application do @impl true def start(_type, _args) do check_and_update_timezone() + attach_oban_telemetry() + Logger.add_handlers(:pinchflat) - children = [ + # See https://hexdocs.pm/elixir/Supervisor.html + # for other strategies and supported options + [ Pinchflat.PromEx, PinchflatWeb.Telemetry, Pinchflat.Repo, @@ -24,17 +28,11 @@ defmodule Pinchflat.Application do {Finch, name: Pinchflat.Finch}, # Start a worker by calling: Pinchflat.Worker.start_link(arg) # {Pinchflat.Worker, arg}, - # Start to serve requests, typically the last entry - PinchflatWeb.Endpoint + # Start to serve requests, typically the last entry (except for the post-boot tasks) + PinchflatWeb.Endpoint, + Pinchflat.Boot.PostBootStartupTasks ] - - attach_oban_telemetry() - Logger.add_handlers(:pinchflat) - - # See https://hexdocs.pm/elixir/Supervisor.html - # for other strategies and supported options - opts = [strategy: :one_for_one, name: Pinchflat.Supervisor] - Supervisor.start_link(children, opts) + |> Supervisor.start_link(strategy: :one_for_one, name: Pinchflat.Supervisor) end # Tell Phoenix to update the endpoint configuration diff --git a/lib/pinchflat/boot/post_boot_startup_tasks.ex b/lib/pinchflat/boot/post_boot_startup_tasks.ex new file mode 100644 index 0000000..d6ae6eb --- /dev/null +++ b/lib/pinchflat/boot/post_boot_startup_tasks.ex @@ -0,0 +1,46 @@ +defmodule Pinchflat.Boot.PostBootStartupTasks do + @moduledoc """ + This module is responsible for running startup tasks on app boot + AFTER all other boot steps have taken place and the app is ready to serve requests. + + It's a GenServer because that plays REALLY nicely with the existing + Phoenix supervision tree. + """ + + alias Pinchflat.YtDlp.UpdateWorker, as: YtDlpUpdateWorker + + # restart: :temporary means that this process will never be restarted (ie: will run once and then die) + use GenServer, restart: :temporary + import Ecto.Query, warn: false + + def start_link(opts \\ []) do + GenServer.start_link(__MODULE__, %{env: Application.get_env(:pinchflat, :env)}, opts) + end + + @doc """ + Runs post-boot application startup tasks. + + Any code defined here will run every time the application starts. You must + make sure that the code is idempotent and safe to run multiple times. + + This is a good place to set up default settings, create initial records, stuff like that. + Should be fast - anything with the potential to be slow should be kicked off as a job instead. + """ + @impl true + def init(%{env: :test} = state) do + # Do nothing _as part of the app bootup process_. + # Since bootup calls `start_link` and that's where the `env` state is injected, + # you can still call `.init()` manually to run these tasks for testing purposes + {:ok, state} + end + + def init(state) do + update_yt_dlp() + + {:ok, state} + end + + defp update_yt_dlp do + YtDlpUpdateWorker.kickoff() + end +end diff --git a/lib/pinchflat/boot/post_job_startup_tasks.ex b/lib/pinchflat/boot/post_job_startup_tasks.ex index 5043a25..6eba701 100644 --- a/lib/pinchflat/boot/post_job_startup_tasks.ex +++ b/lib/pinchflat/boot/post_job_startup_tasks.ex @@ -1,7 +1,7 @@ defmodule Pinchflat.Boot.PostJobStartupTasks do @moduledoc """ This module is responsible for running startup tasks on app boot - AFTER the job runner has initiallized. + AFTER the job runner has initialized. It's a GenServer because that plays REALLY nicely with the existing Phoenix supervision tree. @@ -12,7 +12,7 @@ defmodule Pinchflat.Boot.PostJobStartupTasks do import Ecto.Query, warn: false def start_link(opts \\ []) do - GenServer.start_link(__MODULE__, %{}, opts) + GenServer.start_link(__MODULE__, %{env: Application.get_env(:pinchflat, :env)}, opts) end @doc """ @@ -25,6 +25,13 @@ defmodule Pinchflat.Boot.PostJobStartupTasks do Should be fast - anything with the potential to be slow should be kicked off as a job instead. """ @impl true + def init(%{env: :test} = state) do + # Do nothing _as part of the app bootup process_. + # Since bootup calls `start_link` and that's where the `env` state is injected, + # you can still call `.init()` manually to run these tasks for testing purposes + {:ok, state} + end + def init(state) do # Nothing at the moment! diff --git a/lib/pinchflat/boot/pre_job_startup_tasks.ex b/lib/pinchflat/boot/pre_job_startup_tasks.ex index 2843db9..5035e35 100644 --- a/lib/pinchflat/boot/pre_job_startup_tasks.ex +++ b/lib/pinchflat/boot/pre_job_startup_tasks.ex @@ -19,7 +19,7 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do alias Pinchflat.Lifecycle.UserScripts.CommandRunner, as: UserScriptRunner def start_link(opts \\ []) do - GenServer.start_link(__MODULE__, %{}, opts) + GenServer.start_link(__MODULE__, %{env: Application.get_env(:pinchflat, :env)}, opts) end @doc """ @@ -32,6 +32,13 @@ defmodule Pinchflat.Boot.PreJobStartupTasks do Should be fast - anything with the potential to be slow should be kicked off as a job instead. """ @impl true + def init(%{env: :test} = state) do + # Do nothing _as part of the app bootup process_. + # Since bootup calls `start_link` and that's where the `env` state is injected, + # you can still call `.init()` manually to run these tasks for testing purposes + {:ok, state} + end + def init(state) do ensure_tmpfile_directory() reset_executing_jobs() diff --git a/lib/pinchflat/profiles/media_profile_deletion_worker.ex b/lib/pinchflat/profiles/media_profile_deletion_worker.ex index 529474d..230a085 100644 --- a/lib/pinchflat/profiles/media_profile_deletion_worker.ex +++ b/lib/pinchflat/profiles/media_profile_deletion_worker.ex @@ -14,7 +14,7 @@ defmodule Pinchflat.Profiles.MediaProfileDeletionWorker do Starts the profile deletion worker. Does not attach it to a task like `kickoff_with_task/2` since deletion also cancels all tasks for the profile - Returns {:ok, %Task{}} | {:error, %Ecto.Changeset{}} + Returns {:ok, %Oban.Job{}} | {:error, %Ecto.Changeset{}} """ def kickoff(profile, job_args \\ %{}, job_opts \\ []) do %{id: profile.id} diff --git a/lib/pinchflat/yt_dlp/command_runner.ex b/lib/pinchflat/yt_dlp/command_runner.ex index a2454a8..c90511b 100644 --- a/lib/pinchflat/yt_dlp/command_runner.ex +++ b/lib/pinchflat/yt_dlp/command_runner.ex @@ -76,6 +76,24 @@ defmodule Pinchflat.YtDlp.CommandRunner do end end + @doc """ + Updates yt-dlp to the latest version + + Returns {:ok, binary()} | {:error, binary()} + """ + @impl YtDlpCommandRunner + def update do + command = backend_executable() + + case CliUtils.wrap_cmd(command, ["--update"]) do + {output, 0} -> + {:ok, String.trim(output)} + + {output, _} -> + {:error, output} + end + end + defp generate_output_filepath(addl_opts) do case Keyword.get(addl_opts, :output_filepath) do nil -> FSUtils.generate_metadata_tmpfile(:json) diff --git a/lib/pinchflat/yt_dlp/update_worker.ex b/lib/pinchflat/yt_dlp/update_worker.ex new file mode 100644 index 0000000..2d9b43f --- /dev/null +++ b/lib/pinchflat/yt_dlp/update_worker.ex @@ -0,0 +1,44 @@ +defmodule Pinchflat.YtDlp.UpdateWorker do + @moduledoc false + + use Oban.Worker, + queue: :local_data, + tags: ["local_data"] + + require Logger + + alias __MODULE__ + alias Pinchflat.Settings + + @doc """ + Starts the yt-dlp update worker. Does not attach it to a task like `kickoff_with_task/2` + + Returns {:ok, %Oban.Job{}} | {:error, %Ecto.Changeset{}} + """ + def kickoff do + Oban.insert(UpdateWorker.new(%{})) + end + + @doc """ + Updates yt-dlp and saves the version to the settings. + + This worker is scheduled to run via the Oban Cron plugin as well as on app boot. + + Returns :ok + """ + @impl Oban.Worker + def perform(%Oban.Job{}) do + Logger.info("Updating yt-dlp") + + yt_dlp_runner().update() + + {:ok, yt_dlp_version} = yt_dlp_runner().version() + Settings.set(yt_dlp_version: yt_dlp_version) + + :ok + end + + defp yt_dlp_runner do + Application.get_env(:pinchflat, :yt_dlp_runner) + end +end diff --git a/lib/pinchflat/yt_dlp/yt_dlp_command_runner.ex b/lib/pinchflat/yt_dlp/yt_dlp_command_runner.ex index ff1cbcf..e5c770e 100644 --- a/lib/pinchflat/yt_dlp/yt_dlp_command_runner.ex +++ b/lib/pinchflat/yt_dlp/yt_dlp_command_runner.ex @@ -9,4 +9,5 @@ defmodule Pinchflat.YtDlp.YtDlpCommandRunner do @callback run(binary(), atom(), keyword(), binary()) :: {:ok, binary()} | {:error, binary(), integer()} @callback run(binary(), atom(), keyword(), binary(), keyword()) :: {:ok, binary()} | {:error, binary(), integer()} @callback version() :: {:ok, binary()} | {:error, binary()} + @callback update() :: {:ok, binary()} | {:error, binary()} end diff --git a/lib/pinchflat_web/endpoint.ex b/lib/pinchflat_web/endpoint.ex index b93b36e..908699e 100644 --- a/lib/pinchflat_web/endpoint.ex +++ b/lib/pinchflat_web/endpoint.ex @@ -20,7 +20,7 @@ defmodule PinchflatWeb.Endpoint do plug Plug.Static, at: "/", from: :pinchflat, - gzip: Mix.env() == :prod, + gzip: Application.compile_env(:pinchflat, :env) == :prod, only: PinchflatWeb.static_paths() # Code reloading can be explicitly enabled under the diff --git a/test/pinchflat/boot/post_boot_startup_tasks_test.exs b/test/pinchflat/boot/post_boot_startup_tasks_test.exs new file mode 100644 index 0000000..43a7086 --- /dev/null +++ b/test/pinchflat/boot/post_boot_startup_tasks_test.exs @@ -0,0 +1,16 @@ +defmodule Pinchflat.Boot.PostBootStartupTasksTest do + use Pinchflat.DataCase + + alias Pinchflat.YtDlp.UpdateWorker + alias Pinchflat.Boot.PostBootStartupTasks + + describe "update_yt_dlp" do + test "enqueues an update job" do + assert [] = all_enqueued(worker: UpdateWorker) + + PostBootStartupTasks.init(%{}) + + assert [%Oban.Job{}] = all_enqueued(worker: UpdateWorker) + end + end +end diff --git a/test/pinchflat/yt_dlp/command_runner_test.exs b/test/pinchflat/yt_dlp/command_runner_test.exs index 9f8382b..4c55972 100644 --- a/test/pinchflat/yt_dlp/command_runner_test.exs +++ b/test/pinchflat/yt_dlp/command_runner_test.exs @@ -154,6 +154,14 @@ defmodule Pinchflat.YtDlp.CommandRunnerTest do end end + describe "update/0" do + test "adds the update arg" do + assert {:ok, output} = Runner.update() + + assert String.contains?(output, "--update") + end + end + defp wrap_executable(new_executable, fun) do Application.put_env(:pinchflat, :yt_dlp_executable, new_executable) fun.() diff --git a/test/pinchflat/yt_dlp/update_worker_test.exs b/test/pinchflat/yt_dlp/update_worker_test.exs new file mode 100644 index 0000000..fed0510 --- /dev/null +++ b/test/pinchflat/yt_dlp/update_worker_test.exs @@ -0,0 +1,24 @@ +defmodule Pinchflat.YtDlp.UpdateWorkerTest do + use Pinchflat.DataCase + + alias Pinchflat.Settings + alias Pinchflat.YtDlp.UpdateWorker + + describe "perform/1" do + test "calls the yt-dlp runner to update yt-dlp" do + expect(YtDlpRunnerMock, :update, fn -> {:ok, ""} end) + expect(YtDlpRunnerMock, :version, fn -> {:ok, ""} end) + + perform_job(UpdateWorker, %{}) + end + + test "saves the new version to the database" do + expect(YtDlpRunnerMock, :update, fn -> {:ok, ""} end) + expect(YtDlpRunnerMock, :version, fn -> {:ok, "1.2.3"} end) + + perform_job(UpdateWorker, %{}) + + assert {:ok, "1.2.3"} = Settings.get(:yt_dlp_version) + end + end +end