From 6aaa76a03104238c77d55ff52f58990b50a8e9f5 Mon Sep 17 00:00:00 2001 From: Henri BAUDESSON Date: Thu, 21 Dec 2023 11:49:12 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(app)=20making=20download=20file=20a?= =?UTF-8?q?=20redirect?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, the download file is stream through the a FileResponse when peertube request the video file. This commit change the behaviour to a redirect to the file url. Doing so, the server avoid using ressources to stream the file. --- CHANGELOG.md | 4 ++++ ..._runnerjob_domain_alter_runnerjob_error.py | 24 +++++++++++++++++++ .../models.py | 7 +++--- .../socket.py | 1 + .../transcode.py | 10 +++----- .../job_handlers/abstract_job_handler.py | 5 +++- .../vod_hls_transcoding_job_handler.py | 14 +++++++++-- .../utils/transcoding/job_creation.py | 10 ++++---- .../views/runner_job.py | 15 ++++++++---- tests/app/settings.py | 5 ++++ tests/app/storage.py | 7 +++--- tests/app/urls.py | 3 +++ .../test_transcode.py | 7 +++--- .../job_handlers/test_abstract_job_handler.py | 2 ++ .../test_vod_hls_transcoding_job_handler.py | 5 +++- .../utils/transcoding/test_job_creation.py | 14 +++++------ .../runner_job/test_download_runner_job.py | 5 ++-- .../views/video/test_transcode_video.py | 2 +- .../views/video/test_upload_video.py | 2 +- 19 files changed, 98 insertions(+), 44 deletions(-) create mode 100644 src/django_peertube_runner_connector/migrations/0003_runnerjob_domain_alter_runnerjob_error.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 41088b8..33dc2f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to ## [Unreleased] +## Changed + +- Download video is now a redirect + ## Fixed - Crash when properties were missing from probe diff --git a/src/django_peertube_runner_connector/migrations/0003_runnerjob_domain_alter_runnerjob_error.py b/src/django_peertube_runner_connector/migrations/0003_runnerjob_domain_alter_runnerjob_error.py new file mode 100644 index 0000000..86e562c --- /dev/null +++ b/src/django_peertube_runner_connector/migrations/0003_runnerjob_domain_alter_runnerjob_error.py @@ -0,0 +1,24 @@ +# Generated by Django 5.0 on 2023-12-21 10:34 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("django_peertube_runner_connector", "0002_video_basefilename"), + ] + + operations = [ + migrations.AddField( + model_name="runnerjob", + name="domain", + field=models.CharField( + blank=True, help_text="Job domain", max_length=255, null=True + ), + ), + migrations.AlterField( + model_name="runnerjob", + name="error", + field=models.TextField(blank=True, help_text="Error message", null=True), + ), + ] diff --git a/src/django_peertube_runner_connector/models.py b/src/django_peertube_runner_connector/models.py index 2983f79..a05ac08 100644 --- a/src/django_peertube_runner_connector/models.py +++ b/src/django_peertube_runner_connector/models.py @@ -107,6 +107,9 @@ class RunnerJob(models.Model): default=uuid4, ) uuid = models.UUIDField(unique=True, default=uuid4, help_text="Job UUID") + domain = models.CharField( + max_length=255, null=True, blank=True, help_text="Job domain" + ) type = models.CharField( max_length=255, choices=RunnerJobType.choices, help_text="Job type" ) @@ -116,9 +119,7 @@ class RunnerJob(models.Model): ) state = models.IntegerField(choices=RunnerJobState.choices, help_text="Job state") failures = models.IntegerField(default=0, help_text="Number of failures") - error = models.CharField( - max_length=255, null=True, blank=True, help_text="Error message" - ) + error = models.TextField(null=True, blank=True, help_text="Error message") priority = models.IntegerField(help_text="Job priority") processingJobToken = models.CharField( max_length=255, null=True, blank=True, help_text="Processing job token" diff --git a/src/django_peertube_runner_connector/socket.py b/src/django_peertube_runner_connector/socket.py index 1472973..4fd8918 100644 --- a/src/django_peertube_runner_connector/socket.py +++ b/src/django_peertube_runner_connector/socket.py @@ -45,4 +45,5 @@ async def disconnect(sid): async def send_available_jobs_ping_to_runners(): """Send an "available jobs" ping to the runners.""" + logger.info("Available jobs ping sent to runners") await sio.emit("available-jobs", namespace="/runners") diff --git a/src/django_peertube_runner_connector/transcode.py b/src/django_peertube_runner_connector/transcode.py index 240988b..c3e3ded 100644 --- a/src/django_peertube_runner_connector/transcode.py +++ b/src/django_peertube_runner_connector/transcode.py @@ -1,8 +1,6 @@ """Base function to start the transcoding process.""" import logging -from django.urls import reverse - import ffmpeg from django_peertube_runner_connector.models import Video @@ -23,7 +21,7 @@ class VideoNotFoundError(Exception): """Exception class for when transcoding cannot find a video in the storage.""" -def _process_transcoding(video: Video, video_path: str, video_url: str): +def _process_transcoding(video: Video, video_path: str, domain: str): """ Create a video_file, thumbnails and transcoding jobs for a video. The request will be used to build the video download url. @@ -44,7 +42,7 @@ def _process_transcoding(video: Video, video_path: str, video_url: str): video=video, video_file=video_file, existing_probe=probe, - video_url=video_url, + domain=domain, ) @@ -75,8 +73,6 @@ def transcode_video( baseFilename=base_name, ) - video_url = domain + reverse("runner-jobs-download_video_file", args=(video.uuid,)) - - _process_transcoding(video=video, video_path=file_path, video_url=video_url) + _process_transcoding(video=video, video_path=file_path, domain=domain) return video diff --git a/src/django_peertube_runner_connector/utils/job_handlers/abstract_job_handler.py b/src/django_peertube_runner_connector/utils/job_handlers/abstract_job_handler.py index 30d2b65..17f2678 100644 --- a/src/django_peertube_runner_connector/utils/job_handlers/abstract_job_handler.py +++ b/src/django_peertube_runner_connector/utils/job_handlers/abstract_job_handler.py @@ -28,11 +28,13 @@ class AbstractJobHandler(ABC): """Base class for job handlers.""" @abstractmethod - def create(self, video: Video, resolution, fps, depends_on_runner_job, video_url): + def create(self, video: Video, resolution, fps, depends_on_runner_job, domain): """This method should be implemented by subclasses.""" + # pylint: disable=too-many-arguments def create_runner_job( self, + domain: str, job_type: RunnerJobType, job_uuid: UUID, payload: dict, @@ -43,6 +45,7 @@ def create_runner_job( """This method creates a RunnerJob and send a ping to the runners.""" runner_job = RunnerJob.objects.create( type=job_type, + domain=domain, payload=payload, privatePayload=private_payload, uuid=job_uuid, diff --git a/src/django_peertube_runner_connector/utils/job_handlers/vod_hls_transcoding_job_handler.py b/src/django_peertube_runner_connector/utils/job_handlers/vod_hls_transcoding_job_handler.py index 20b2a2d..e8c5169 100644 --- a/src/django_peertube_runner_connector/utils/job_handlers/vod_hls_transcoding_job_handler.py +++ b/src/django_peertube_runner_connector/utils/job_handlers/vod_hls_transcoding_job_handler.py @@ -5,6 +5,8 @@ import os import uuid +from django.urls import reverse + from django_peertube_runner_connector.models import RunnerJob, RunnerJobType, Video from django_peertube_runner_connector.storage import video_storage from django_peertube_runner_connector.utils.files import ( @@ -30,12 +32,19 @@ class VODHLSTranscodingJobHandler(AbstractVODTranscodingJobHandler): """Handler for vod hls transcoding jobs.""" - def create(self, video: Video, resolution, fps, depends_on_runner_job, video_url): + def create(self, video: Video, resolution, fps, depends_on_runner_job, domain: str): job_uuid = uuid.uuid4() payload = { "input": { - "videoFileUrl": video_url, + "videoFileUrl": domain + + reverse( + "runner-jobs-download_video_file", + args=( + video.uuid, + job_uuid, + ), + ), }, "output": { "resolution": resolution, @@ -50,6 +59,7 @@ def create(self, video: Video, resolution, fps, depends_on_runner_job, video_url } job = self.create_runner_job( + domain=domain, job_type=RunnerJobType.VOD_HLS_TRANSCODING, job_uuid=job_uuid, payload=payload, diff --git a/src/django_peertube_runner_connector/utils/transcoding/job_creation.py b/src/django_peertube_runner_connector/utils/transcoding/job_creation.py index 4c1ae3b..f74973c 100644 --- a/src/django_peertube_runner_connector/utils/transcoding/job_creation.py +++ b/src/django_peertube_runner_connector/utils/transcoding/job_creation.py @@ -77,7 +77,7 @@ def build_lower_resolution_job_payloads( input_video_fps, has_audio, main_runner_job, - video_url, + domain: str, ): """Build lower resolution runner job.""" resolutions_enabled = compute_resolutions_to_transcode( @@ -99,12 +99,12 @@ def build_lower_resolution_job_payloads( resolution=resolution, fps=fps, depends_on_runner_job=main_runner_job, - video_url=video_url, + domain=domain, ) def create_transcoding_jobs( - video: Video, video_file: VideoFile, video_url, existing_probe=None + video: Video, video_file: VideoFile, domain: str, existing_probe=None ): """Create transcoding jobs.""" setting_transcoding = get_video_transcoding_fps_settings() @@ -134,7 +134,7 @@ def create_transcoding_jobs( resolution=max_resolution, fps=fps, depends_on_runner_job=None, - video_url=video_url, + domain=domain, ) build_lower_resolution_job_payloads( @@ -143,5 +143,5 @@ def create_transcoding_jobs( input_video_fps=input_fps, has_audio=has_audio, main_runner_job=main_runner_job, - video_url=video_url, + domain=domain, ) diff --git a/src/django_peertube_runner_connector/views/runner_job.py b/src/django_peertube_runner_connector/views/runner_job.py index 12759f9..332a68b 100644 --- a/src/django_peertube_runner_connector/views/runner_job.py +++ b/src/django_peertube_runner_connector/views/runner_job.py @@ -1,8 +1,10 @@ """API Endpoints for Runner Jobs with Django RestFramework viewsets.""" import logging +from urllib.parse import urlparse from uuid import uuid4 -from django.http import FileResponse, Http404 +from django.http import Http404 +from django.shortcuts import redirect from django.utils import timezone from rest_framework import status, viewsets @@ -182,13 +184,14 @@ def success_runner_job(self, request, uuid=None): @action( detail=False, methods=["post"], - url_path="files/videos/(?P[^/.]+)/max-quality", + url_path="files/videos/(?P[^/.]+)/(?P[^/.]+)/max-quality", url_name="download_video_file", ) - def download_video_file(self, request, video_id=None): + def download_video_file(self, request, video_id=None, job_id=None): """Endpoint to download a video file.""" runner = self._get_runner_from_token(request) video = self._get_video_from_uuid(video_id) + job = self._get_job_from_uuid(job_id) logger.info( "Get max quality file of video %s for runner %s", @@ -197,5 +200,7 @@ def download_video_file(self, request, video_id=None): ) video_file = video.get_max_quality_file() - - return FileResponse(video_storage.open(video_file.filename, "rb")) + video_url = video_storage.url(video_file.filename) + if not urlparse(video_url).scheme and job.domain: + video_url = job.domain + video_url + return redirect(video_url, permanent=True) diff --git a/tests/app/settings.py b/tests/app/settings.py index 2872493..f891324 100644 --- a/tests/app/settings.py +++ b/tests/app/settings.py @@ -10,6 +10,7 @@ https://docs.djangoproject.com/en/4.2/ref/settings/ """ +import os from pathlib import Path from configurations import Configuration, values @@ -97,6 +98,10 @@ class Base(Configuration): } STATIC_ROOT = BASE_DIR / "staticfiles" + # Used the serve static files + VIDEOS_ROOT = values.Value(os.path.join(BASE_DIR, "storage")) + # Use to create the endpoint that will serve the static files + VIDEO_URL = "storage" STORAGES = { "default": { diff --git a/tests/app/storage.py b/tests/app/storage.py index 52997b4..389e651 100644 --- a/tests/app/storage.py +++ b/tests/app/storage.py @@ -1,4 +1,5 @@ """Custom storage class for testing.""" +from django.conf import settings from django.core.files.storage import FileSystemStorage from storages.backends.s3boto3 import S3Boto3Storage @@ -13,7 +14,5 @@ class MyS3VideoStorage(S3Boto3Storage): class MyCustomFileSystemVideoStorage(FileSystemStorage): """Custom FileSystemStorage class.""" - def url(self, name): - return self.path(name) - - location = "storage" + location = settings.VIDEO_URL + base_url = f"http://localhost:8000/{settings.VIDEO_URL}/" diff --git a/tests/app/urls.py b/tests/app/urls.py index 01cf1b1..6fc09d8 100644 --- a/tests/app/urls.py +++ b/tests/app/urls.py @@ -1,4 +1,6 @@ """URLs for the test app.""" +from django.conf import settings +from django.conf.urls.static import static from django.contrib import admin from django.contrib.staticfiles.urls import staticfiles_urlpatterns from django.urls import include, path @@ -21,3 +23,4 @@ ] urlpatterns += django_peertube_runner_connector_urls urlpatterns += staticfiles_urlpatterns() +urlpatterns += static(settings.VIDEO_URL, document_root=settings.VIDEOS_ROOT) diff --git a/tests/tests_django_peertube_runner_connector/test_transcode.py b/tests/tests_django_peertube_runner_connector/test_transcode.py index bcabc78..83dd8cc 100644 --- a/tests/tests_django_peertube_runner_connector/test_transcode.py +++ b/tests/tests_django_peertube_runner_connector/test_transcode.py @@ -49,8 +49,7 @@ def test_transcode(self, mock_process): mock_process.assert_called_with( video=created_video, video_path=video_url, - video_url="https://example.com/api/v1/runners/jobs/" - f"files/videos/{created_video.uuid}/max-quality", + domain="https://example.com", ) @patch.object(ffmpeg, "probe") @@ -75,7 +74,7 @@ def test_process_transcoding( _process_transcoding( video=video, video_path=video_url, - video_url="download_video_url", + domain="domain", ) mock_probe.assert_called_with("/test_directory/file.mp4") @@ -90,7 +89,7 @@ def test_process_transcoding( video=video, video_file=video_file, existing_probe=mock_probe.return_value, - video_url="download_video_url", + domain="domain", ) self.assertEqual(video.duration, 900) diff --git a/tests/tests_django_peertube_runner_connector/utils/job_handlers/test_abstract_job_handler.py b/tests/tests_django_peertube_runner_connector/utils/job_handlers/test_abstract_job_handler.py index 5d6d44b..be87cff 100644 --- a/tests/tests_django_peertube_runner_connector/utils/job_handlers/test_abstract_job_handler.py +++ b/tests/tests_django_peertube_runner_connector/utils/job_handlers/test_abstract_job_handler.py @@ -54,6 +54,7 @@ def test_create_runner_parent_job(self, mock_ping): runner_job = handler.create_runner_job( job_type=RunnerJobType.VOD_HLS_TRANSCODING, job_uuid="123e4567-e89b-12d3-a456-426655440003", + domain="domain", payload={"test": "test"}, private_payload={"private": "private"}, priority=0, @@ -78,6 +79,7 @@ def test_create_runner_child_job(self, mock_ping): runner_job = handler.create_runner_job( job_type=RunnerJobType.VOD_HLS_TRANSCODING, job_uuid="123e4567-e89b-12d3-a456-426655440003", + domain="domain", payload={"test": "test"}, private_payload={"private": "private"}, priority=0, diff --git a/tests/tests_django_peertube_runner_connector/utils/job_handlers/test_vod_hls_transcoding_job_handler.py b/tests/tests_django_peertube_runner_connector/utils/job_handlers/test_vod_hls_transcoding_job_handler.py index e3319da..62c9272 100644 --- a/tests/tests_django_peertube_runner_connector/utils/job_handlers/test_vod_hls_transcoding_job_handler.py +++ b/tests/tests_django_peertube_runner_connector/utils/job_handlers/test_vod_hls_transcoding_job_handler.py @@ -50,7 +50,10 @@ def test_create(self): runner_job.payload, { "input": { - "videoFileUrl": "test_url", + "videoFileUrl": ( + "test_url/api/v1/runners/jobs/files/videos/123e4567-e89b-" + f"12d3-a456-426655440002/{runner_job.uuid}/max-quality" + ), }, "output": { "resolution": "720", diff --git a/tests/tests_django_peertube_runner_connector/utils/transcoding/test_job_creation.py b/tests/tests_django_peertube_runner_connector/utils/transcoding/test_job_creation.py index d0a3be5..2f7f193 100644 --- a/tests/tests_django_peertube_runner_connector/utils/transcoding/test_job_creation.py +++ b/tests/tests_django_peertube_runner_connector/utils/transcoding/test_job_creation.py @@ -115,16 +115,14 @@ def test_create_transcoding_jobs(self, mock_build_resolution, mock_job_handler): mocked_class = Mock() mock_job_handler.return_value = mocked_class - create_transcoding_jobs( - self.video, self.video_file, "video_url", probe_response - ) + create_transcoding_jobs(self.video, self.video_file, "domain", probe_response) mocked_class.create.assert_called_with( video=self.video, resolution=540, fps=30, depends_on_runner_job=None, - video_url="video_url", + domain="domain", ) mock_build_resolution.assert_called_with( @@ -133,7 +131,7 @@ def test_create_transcoding_jobs(self, mock_build_resolution, mock_job_handler): input_video_fps=30, has_audio=True, main_runner_job=mocked_class.create(), - video_url="video_url", + domain="domain", ) @patch( @@ -153,7 +151,7 @@ def test_build_lower_resolution_job_payloads(self, mock_job_handler): input_video_fps=30, has_audio=True, main_runner_job=main_runner_job, - video_url="video_url", + domain="domain", ) mocked_class.create.assert_has_calls( @@ -163,14 +161,14 @@ def test_build_lower_resolution_job_payloads(self, mock_job_handler): resolution=360, fps=30, depends_on_runner_job=main_runner_job, - video_url="video_url", + domain="domain", ), call( video=self.video, resolution=480, fps=30, depends_on_runner_job=main_runner_job, - video_url="video_url", + domain="domain", ), ] ) diff --git a/tests/tests_django_peertube_runner_connector/views/runner_job/test_download_runner_job.py b/tests/tests_django_peertube_runner_connector/views/runner_job/test_download_runner_job.py index e2e8f3e..cf24289 100644 --- a/tests/tests_django_peertube_runner_connector/views/runner_job/test_download_runner_job.py +++ b/tests/tests_django_peertube_runner_connector/views/runner_job/test_download_runner_job.py @@ -97,10 +97,11 @@ def test_download_video_with_a_valid_runner_token(self): response = self.client.post( "/api/v1/runners/jobs/" - "files/videos/02404b18-3c50-4929-af61-913f4df65e99/max-quality", + "files/videos/02404b18-3c50-4929-af61-913f4df65e99/" + "02404b18-3c50-4929-af61-913f4df65e00/max-quality", data={ "runnerToken": "runnerToken", }, ) - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, 301) diff --git a/tests/tests_django_peertube_runner_connector/views/video/test_transcode_video.py b/tests/tests_django_peertube_runner_connector/views/video/test_transcode_video.py index a18d606..6ee95e3 100644 --- a/tests/tests_django_peertube_runner_connector/views/video/test_transcode_video.py +++ b/tests/tests_django_peertube_runner_connector/views/video/test_transcode_video.py @@ -102,5 +102,5 @@ def test_transcode_video(self): self.assertEqual( runner_job.payload["input"]["videoFileUrl"], "http://testserver/api/v1/runners/jobs/" - f"files/videos/{created_video.uuid}/max-quality", + f"files/videos/{created_video.uuid}/{runner_job.uuid}/max-quality", ) diff --git a/tests/tests_django_peertube_runner_connector/views/video/test_upload_video.py b/tests/tests_django_peertube_runner_connector/views/video/test_upload_video.py index 8b6e2e0..104dafa 100644 --- a/tests/tests_django_peertube_runner_connector/views/video/test_upload_video.py +++ b/tests/tests_django_peertube_runner_connector/views/video/test_upload_video.py @@ -89,5 +89,5 @@ def test_upload_video(self): self.assertEqual( runner_job.payload["input"]["videoFileUrl"], "http://testserver/api/v1/runners/jobs/" - f"files/videos/{created_video.uuid}/max-quality", + f"files/videos/{created_video.uuid}/{runner_job.uuid}/max-quality", )