HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythondjangoMinor

Converting an already-uploaded file and saving it to a model's FileField

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filefieldfilealreadyuploadedandconvertingsavingmodel

Problem

Please review this:

from os import path, remove

try:
    video = Video.objects.get(id=original_video_id)
except ObjectDoesNotExist:
    return False

convert_command = ['ffmpeg', '-i', input_file, '-acodec', 
       'libmp3lame', '-y', '-ac', '2', '-ar', 
       '44100', '-aq', '5', '-qscale', '10', 
       '%s.flv' % output_file]                     

convert_system_call = subprocess.Popen(
    convert_command,
    stderr=subprocess.STDOUT,
    stdout=subprocess.PIPE
)

logger.debug(convert_system_call.stdout.read())

try:

    f = open('%s.flv' % output_file, 'r')
    filecontent = ContentFile(f.read())
    video.converted_video.save('%s.flv' % output_file, 
                     filecontent, 
                     save=True)
    f.close()

    remove('%s.flv' % output_file)

    video.save()

    return True
except:
    return False

Solution

Clean, easy to understand code. However:

Never hide errors with a bare except. Change

try:
    ...
except:
    return False


into

try:
    ...
except (IOError, AnyOther, ExceptionThat, YouExpect):
    logging.exception("File conversion failed")


Also, unless you need to support Python 2.4 or earlier, you want to use the files context manager support:

with open('%s.flv' % output_file, 'r') as f:
    filecontent = ContentFile(f.read())
    video.converted_video.save('%s.flv' % output_file, 
                               filecontent, 
                               save=True)
remove('%s.flv' % output_file)


That way the file will be closed immediately after exiting the with-block, even if there is an error. For Python 2.5 you would have to from __future__ import with_statement as well.

You might also want to look at using a temporary file from tempfile for the output file.

Code Snippets

try:
    ...
except:
    return False
try:
    ...
except (IOError, AnyOther, ExceptionThat, YouExpect):
    logging.exception("File conversion failed")
with open('%s.flv' % output_file, 'r') as f:
    filecontent = ContentFile(f.read())
    video.converted_video.save('%s.flv' % output_file, 
                               filecontent, 
                               save=True)
remove('%s.flv' % output_file)

Context

StackExchange Code Review Q#507, answer score: 5

Revisions (0)

No revisions yet.