refactor: auto-remove temporary files

video_encoding/files.py

@@ -1,10 +1,7 @@
-import os
-
 from django.core.files import File
 
-from video_encoding.utils import get_fieldfile_local_path
-
 from .backends import get_backend
+from .utils import get_local_path
 
 
 class VideoFile(File):
@@ -44,12 +41,8 @@ def _get_video_info(self):
         if not hasattr(self, '_info_cache'):
             encoding_backend = get_backend()
 
-            local_path, local_tmp_file = get_fieldfile_local_path(fieldfile=self)
-
-            info_cache = encoding_backend.get_media_info(local_path)
-
-            if local_tmp_file:
-                os.unlink(local_tmp_file.name)
+            with get_local_path(self) as local_path:
+                info_cache = encoding_backend.get_media_info(local_path)
 
             self._info_cache = info_cache
 

video_encoding/tasks.py

@@ -5,13 +5,13 @@
 from django.contrib.contenttypes.models import ContentType
 from django.core.files import File
 
-from video_encoding.utils import get_fieldfile_local_path
-
 from .backends import get_backend
+from .backends.base import BaseEncodingBackend
 from .config import settings
 from .exceptions import VideoEncodingError
 from .fields import VideoField
 from .models import Format
+from .utils import get_local_path
 
 
 def convert_all_videos(app_label, model_name, object_pk):
@@ -42,60 +42,64 @@ def convert_video(fieldfile, force=False):
     instance = fieldfile.instance
     field = fieldfile.field
 
-    local_path, temp_file = get_fieldfile_local_path(fieldfile=fieldfile)
+    with get_local_path(fieldfile) as source_path:
 
-    filename = os.path.basename(local_path)
-    source_path = local_path
+        encoding_backend = get_backend()
 
-    encoding_backend = get_backend()
+        for options in settings.VIDEO_ENCODING_FORMATS[encoding_backend.name]:
+            video_format, created = Format.objects.get_or_create(
+                object_id=instance.pk,
+                content_type=ContentType.objects.get_for_model(instance),
+                field_name=field.name,
+                format=options['name'],
+            )
 
-    for options in settings.VIDEO_ENCODING_FORMATS[encoding_backend.name]:
-        video_format, created = Format.objects.get_or_create(
-            object_id=instance.pk,
-            content_type=ContentType.objects.get_for_model(instance),
-            field_name=field.name,
-            format=options['name'],
-        )
+            # do not reencode if not requested
+            if video_format.file and not force:
+                continue
 
-        # do not reencode if not requested
-        if video_format.file and not force:
-            continue
-        else:
-            # set progress to 0
-            video_format.reset_progress()
+            try:
+                _encode(source_path, video_format, encoding_backend, options)
+            except VideoEncodingError:
+                # TODO handle with more care
+                video_format.delete()
+                continue
 
-        # TODO do not upscale videos
 
-        _, target_path = tempfile.mkstemp(
-            suffix='_{name}.{extension}'.format(**options)
-        )
+def _encode(
+    source_path: str,
+    video_format: Format,
+    encoding_backend: BaseEncodingBackend,
+    options: dict,
+) -> None:
+    """
+    Encode video and continously report encoding progress.
+    """
+    # TODO do not upscale videos
+    # TODO move logic git VideoFormat class
 
-        try:
-            encoding = encoding_backend.encode(
-                source_path, target_path, options['params']
-            )
-            while encoding:
-                try:
-                    progress = next(encoding)
-                except StopIteration:
-                    break
-                video_format.update_progress(progress)
-        except VideoEncodingError:
-            # TODO handle with more care
-            video_format.delete()
-            os.remove(target_path)
-            continue
+    with tempfile.NamedTemporaryFile(
+        suffix='_{name}.{extension}'.format(**options)
+    ) as file_handler:
+        target_path = file_handler.name
+
+        # set progress to 0
+        video_format.reset_progress()
+
+        encoding = encoding_backend.encode(source_path, target_path, options['params'])
+        while encoding:
+            try:
+                progress = next(encoding)
+            except StopIteration:
+                break
+            video_format.update_progress(progress)
 
         # save encoded file
+        filename = os.path.basename(source_path)
+        # TODO remove existing file?
         video_format.file.save(
             '{filename}_{name}.{extension}'.format(filename=filename, **options),
             File(open(target_path, mode='rb')),
         )
 
         video_format.update_progress(100)  # now we are ready
-
-        # remove temporary file
-        os.remove(target_path)
-
-    if temp_file:
-        os.unlink(temp_file.name)

video_encoding/utils.py

@@ -1,31 +1,30 @@
+import contextlib
 import os
 import tempfile
+from typing import Generator
 
+from django.core.files import File
 
-def get_fieldfile_local_path(fieldfile):
-    local_temp_file = None
 
-    if hasattr(fieldfile, 'storage'):
+@contextlib.contextmanager
+def get_local_path(fieldfile: File) -> Generator[str, None, None]:
+    if not hasattr(fieldfile, 'storage'):
+        # Its a local file with no storage abstraction
+        try:
+            yield os.path.abspath(fieldfile.path)
+        except AttributeError:
+            yield os.path.abspath(fieldfile.name)
+    else:
         storage = fieldfile.storage
         try:
             # Try to access with path
-            storage_local_path = storage.path(fieldfile.path)
+            yield storage.path(fieldfile.path)
         except (NotImplementedError, AttributeError):
-            # Storage doesnt support absolute paths, download
-            # file to a temp local dir
-            storage_file = storage.open(fieldfile.name, 'rb')
-
-            local_temp_file = tempfile.NamedTemporaryFile(delete=False)
-            local_temp_file.write(storage_file.read())
-            local_temp_file.close()
-            storage_local_path = local_temp_file.name
-    else:
-        # Its a local file with no storage abstraction
-        try:
-            path = os.path.abspath(fieldfile.path)
-        except AttributeError:
-            path = os.path.abspath(fieldfile.name)
-
-        storage_local_path = path
+            # Storage doesnt support absolute paths,
+            # download file to a temp local dir
+            with tempfile.NamedTemporaryFile(delete=False) as temp_file:
+                storage_file = storage.open(fieldfile.name, 'r')
 
-    return storage_local_path, local_temp_file
+                temp_file.write(storage_file.read())
+                temp_file.flush()
+                yield temp_file.name