From 6b9ee0ca13b99ee20f9d0c680a950c6a7494a5a0 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 24 Jul 2011 23:12:46 -0500 Subject: [PATCH 01/14] Store the task id of a processing action in the database. --- mediagoblin/db/migrations.py | 12 ++++++++++++ mediagoblin/db/models.py | 3 +++ mediagoblin/submit/views.py | 3 ++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index 6a8ebcf9..797d39de 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -52,3 +52,15 @@ def mediaentry_mediafiles_main_to_original(database): document['media_files']['original'] = original collection.save(document) + + +@RegisterMigration(3) +def mediaentry_add_queued_task_id(database): + """ + Add the 'queued_task_id' field for entries that don't have it. + """ + collection = database['media_entries'] + collection.update( + {'queued_task_id': {'$exists': False}}, + {'$set': {'queued_task_id': None}}, + multi=True) diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index bad15aca..e97dc537 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -162,6 +162,8 @@ class MediaEntry(Document): queued for processing. This is stored in the mg_globals.queue_store storage system. + - queued_task_id: celery task id. Use this to fetch the task state. + - media_files: Files relevant to this that have actually been processed and are available for various types of display. Stored like: {'thumb': ['dir1', 'dir2', 'pic.png'} @@ -190,6 +192,7 @@ class MediaEntry(Document): # For now let's assume there can only be one main file queued # at a time 'queued_media_file': [unicode], + 'queued_task_id': unicode, # A dictionary of logical names to filepaths 'media_files': dict, diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index 1848f5e5..f19bf22e 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -84,7 +84,8 @@ def submit_start(request): entry.save(validate=True) # queue it for processing - process_media_initial.delay(unicode(entry['_id'])) + result = process_media_initial.delay(unicode(entry['_id'])) + entry['queued_task_id'] = result.task_id add_message(request, SUCCESS, 'Woohoo! Submitted!') From d990a3799846a06ada11805ca4f2806eb1bf8b5e Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Wed, 10 Aug 2011 20:26:22 -0500 Subject: [PATCH 02/14] We should save the entry *after* we add the queued_task_id. --- mediagoblin/submit/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index 59d8fe3f..53711236 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -88,11 +88,11 @@ def submit_start(request): # Add queued filename to the entry entry['queued_media_file'] = queue_filepath - entry.save(validate=True) # queue it for processing result = process_media_initial.delay(unicode(entry['_id'])) entry['queued_task_id'] = result.task_id + entry.save(validate=True) add_message(request, SUCCESS, _('Woohoo! Submitted!')) From 07934b442f7cd3abae18eecdf533de004f88e6b1 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Thu, 11 Aug 2011 11:30:26 -0500 Subject: [PATCH 03/14] Moving things around a bit/commenting in the submit view to make the workflow clearer --- mediagoblin/submit/views.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index d4858c87..1e8c6a68 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -87,9 +87,13 @@ def submit_start(request): # Add queued filename to the entry entry['queued_media_file'] = queue_filepath - # queue it for processing + # Save now so we have this data before kicking off processing + entry.save(validate=False) + result = process_media_initial.delay(unicode(entry['_id'])) - entry['queued_task_id'] = result.task_id + + # Save the task id + entry['queued_task_id'] = unicode(result.task_id) entry.save(validate=True) add_message(request, SUCCESS, _('Woohoo! Submitted!')) From fabdccd0114cc75f2191419dcd708d7858718a45 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Fri, 12 Aug 2011 13:14:35 -0500 Subject: [PATCH 04/14] Missing multi=True closing this migration, oops :) --- mediagoblin/db/migrations.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index 36bca5b3..171b5c83 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -62,6 +62,7 @@ def mediaentry_remove_thumbnail_file(database): database['media_entries'].update( {'thumbnail_file': {'$exists': True}}, {'$unset': {'thumbnail_file': 1}}, + multi=True) @RegisterMigration(4) From 4b860cb823fd160742ab050f481eb65e389f9a7b Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Fri, 12 Aug 2011 19:59:19 -0500 Subject: [PATCH 05/14] Create processing errors and raise BadMediaFail on failure to load the image --- mediagoblin/process_media/__init__.py | 8 +++- mediagoblin/process_media/errors.py | 54 +++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 mediagoblin/process_media/errors.py diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index 8e12ca4d..00402d7e 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -21,6 +21,8 @@ from celery.task import task from mediagoblin import mg_globals as mgg from contextlib import contextmanager +from mediagoblin.process_media.errors import BadMediaFail + THUMB_SIZE = 180, 180 MEDIUM_SIZE = 640, 640 @@ -51,7 +53,11 @@ def process_media_initial(media_id): mgg.queue_store, queued_filepath, 'source') - thumb = Image.open(queued_filename) + try: + thumb = Image.open(queued_filename) + except IOError: + raise BadMediaFail() + thumb.thumbnail(THUMB_SIZE, Image.ANTIALIAS) # ensure color mode is compatible with jpg if thumb.mode != "RGB": diff --git a/mediagoblin/process_media/errors.py b/mediagoblin/process_media/errors.py new file mode 100644 index 00000000..f2ae87ff --- /dev/null +++ b/mediagoblin/process_media/errors.py @@ -0,0 +1,54 @@ +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011 Free Software Foundation, Inc +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +from mediagoblin.util import lazy_pass_to_ugettext as _ + +class BaseProcessingFail(Exception): + """ + Base exception that all other processing failure messages should + subclass from. + + You shouldn't call this itself; instead you should subclass it + and provid the exception_path and general_message applicable to + this error. + """ + general_message = u'' + + @property + def exception_path(self): + return u"%s.%s" % ( + self.__class__.__module__, self.__class__.__name__) + + def __init__(self, **metadata): + self.metadata = metadata or {} + + def generate_error_message(self): + """ + Generate an error to display to users in the panel. + + Uses this class's general_message, possibly interpolated + with any metadata in self.metadata['error_message_vars'], + if appropriate. + """ + return self.general_message % self.metadata.get('error_message_vars', {}) + + +class BadMediaFail(BaseProcessingFail): + """ + Error that should be raised when an inappropriate file was given + for the media type specified. + """ + general_message = _(u'Invalid file given for media type.') From 6c50c2106816c920ef404dea641a8eac8c5914eb Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 13 Aug 2011 07:48:34 -0500 Subject: [PATCH 06/14] Add fail_error and fail_metadata fields to MediaEntry and relevant migration --- mediagoblin/db/migrations.py | 17 +++++++++++++++++ mediagoblin/db/models.py | 10 +++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index 171b5c83..5456b248 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -75,3 +75,20 @@ def mediaentry_add_queued_task_id(database): {'queued_task_id': {'$exists': False}}, {'$set': {'queued_task_id': None}}, multi=True) + + +@RegisterMigration(5) +def mediaentry_add_fail_error_and_metadata(database): + """ + Add 'fail_error' and 'fail_metadata' fields to media entries + """ + collection = database['media_entries'] + collection.update( + {'fail_error': {'$exists': False}}, + {'$set': {'fail_error': None}}, + multi=True) + + collection.update( + {'fail_metadata': {'$exists': False}}, + {'$set': {'fail_metadata': {}}}, + multi=True) diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 0dcb6ce8..982883d7 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -171,6 +171,9 @@ class MediaEntry(Document): - attachment_files: A list of "attachment" files, ones that aren't critical to this piece of media but may be usefully relevant to people viewing the work. (currently unused.) + + - fail_error: path to the exception raised + - fail_metadata: """ __collection__ = 'media_entries' @@ -197,7 +200,12 @@ class MediaEntry(Document): # The following should be lists of lists, in appropriate file # record form - 'attachment_files': list} + 'attachment_files': list, + + # If things go badly in processing things, we'll store that + # data here + 'fail_error': unicode, + 'fail_metadata': dict} required_fields = [ 'uploader', 'created', 'media_type', 'slug'] From 4a477e246d07a4c26f084db2596caf3310b78609 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 13 Aug 2011 10:59:34 -0500 Subject: [PATCH 07/14] Proper handling of processor failures, working as hoped! BaseProcessingFail based exceptions recorded and marked appropriately in the database. Other exceptions also caught and marked (or rather not marked) appropriately in the database as well. --- mediagoblin/process_media/__init__.py | 76 +++++++++++++++++++++++---- mediagoblin/submit/views.py | 26 ++++++--- 2 files changed, 83 insertions(+), 19 deletions(-) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index 00402d7e..d6cdd747 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -15,13 +15,14 @@ # along with this program. If not, see . import Image -from mediagoblin.db.util import ObjectId -from celery.task import task -from mediagoblin import mg_globals as mgg from contextlib import contextmanager +from celery.task import task, Task +from celery import registry -from mediagoblin.process_media.errors import BadMediaFail +from mediagoblin.db.util import ObjectId +from mediagoblin import mg_globals as mgg +from mediagoblin.process_media.errors import BaseProcessingFail, BadMediaFail THUMB_SIZE = 180, 180 @@ -34,6 +35,7 @@ def create_pub_filepath(entry, filename): unicode(entry['_id']), filename]) + @contextmanager def closing(callback): try: @@ -41,12 +43,66 @@ def closing(callback): finally: pass -@task -def process_media_initial(media_id): - workbench = mgg.workbench_manager.create_workbench() - entry = mgg.database.MediaEntry.one( - {'_id': ObjectId(media_id)}) +################################ +# Media processing initial steps +################################ + +class ProcessMedia(Task): + """ + Pass this entry off for processing. + """ + def run(self, media_id): + """ + Pass the media entry off to the appropriate processing function + (for now just process_image...) + """ + entry = mgg.database.MediaEntry.one( + {'_id': ObjectId(media_id)}) + process_image(entry) + entry['state'] = u'processed' + entry.save() + + def on_failure(self, exc, task_id, args, kwargs, einfo): + """ + If the processing failed we should mark that in the database. + + Assuming that the exception raised is a subclass of BaseProcessingFail, + we can use that to get more information about the failure and store that + for conveying information to users about the failure, etc. + """ + media_id = args[0] + entry = mgg.database.MediaEntry.one( + {'_id': ObjectId(media_id)}) + + entry[u'state'] = u'failed' + + # Was this a BaseProcessingFail? In other words, was this a + # type of error that we know how to handle? + if isinstance(exc, BaseProcessingFail): + # Looks like yes, so record information about that failure and any + # metadata the user might have supplied. + entry[u'fail_error'] = exc.exception_path + entry[u'fail_metadata'] = exc.metadata + else: + # Looks like no, so just mark it as failed and don't record a + # failure_error (we'll assume it wasn't handled) and don't record + # metadata (in fact overwrite it if somehow it had previous info + # here) + entry[u'fail_error'] = None + entry[u'fail_metadata'] = {} + + entry.save() + + +process_media = registry.tasks[ProcessMedia.name] + + +def process_image(entry): + """ + Code to process an image + """ + workbench = mgg.workbench_manager.create_workbench() queued_filepath = entry['queued_media_file'] queued_filename = workbench.localized_file( @@ -107,8 +163,6 @@ def process_media_initial(media_id): media_files_dict['original'] = original_filepath if medium_processed: media_files_dict['medium'] = medium_filepath - entry['state'] = u'processed' - entry.save() # clean up workbench workbench.destroy_self() diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index 1e8c6a68..25b3664b 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -14,9 +14,10 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import uuid + from os.path import splitext from cgi import FieldStorage -from string import split from werkzeug.utils import secure_filename @@ -27,7 +28,7 @@ from mediagoblin.util import ( from mediagoblin.util import pass_to_ugettext as _ from mediagoblin.decorators import require_active_login from mediagoblin.submit import forms as submit_forms, security -from mediagoblin.process_media import process_media_initial +from mediagoblin.process_media import process_media from mediagoblin.messages import add_message, SUCCESS @@ -87,15 +88,24 @@ def submit_start(request): # Add queued filename to the entry entry['queued_media_file'] = queue_filepath + # We generate this ourselves so we know what the taks id is for + # retrieval later. + # (If we got it off the task's auto-generation, there'd be a risk of + # a race condition when we'd save after sending off the task) + task_id = unicode(uuid.uuid4()) + entry['queued_task_id'] = task_id + # Save now so we have this data before kicking off processing - entry.save(validate=False) - - result = process_media_initial.delay(unicode(entry['_id'])) - - # Save the task id - entry['queued_task_id'] = unicode(result.task_id) entry.save(validate=True) + # Pass off to processing + # + # (... don't change entry after this point to avoid race + # conditions with changes to the document via processing code) + process_media.apply_async( + [unicode(entry['_id'])], {}, + task_id=task_id) + add_message(request, SUCCESS, _('Woohoo! Submitted!')) return redirect(request, "mediagoblin.user_pages.user_home", From 6788b4123ef00241d6b6c80ca93d655e4307d6e3 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 13 Aug 2011 12:21:06 -0500 Subject: [PATCH 08/14] Capture and properly handle errors. Handled in several places: - In the run() of the ProcessMedia itself for handled (BaseProcessingFail derived) errors (best to do these not in on_failure because the errors are highlighted in celeryd in a way that looks inappropriate for when the errors are well handled) - In ProcessMedia.on_failure() for all other errors - In the submit view where all exceptions are caught, media is marked at having failed, then the error is re-raised. (The reason for this is that users running in "lazy" mode will get errors propagated by celery and so on_failure won't run for them.) --- mediagoblin/process_media/__init__.py | 56 ++++++++++++++++----------- mediagoblin/submit/views.py | 21 ++++++++-- 2 files changed, 50 insertions(+), 27 deletions(-) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index d6cdd747..69e4fc45 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -59,7 +59,14 @@ class ProcessMedia(Task): """ entry = mgg.database.MediaEntry.one( {'_id': ObjectId(media_id)}) - process_image(entry) + + # Try to process, and handle expected errors. + try: + process_image(entry) + except BaseProcessingFail, exc: + mark_entry_failed(entry[u'_id'], exc) + return + entry['state'] = u'processed' entry.save() @@ -71,33 +78,36 @@ class ProcessMedia(Task): we can use that to get more information about the failure and store that for conveying information to users about the failure, etc. """ - media_id = args[0] - entry = mgg.database.MediaEntry.one( - {'_id': ObjectId(media_id)}) - - entry[u'state'] = u'failed' - - # Was this a BaseProcessingFail? In other words, was this a - # type of error that we know how to handle? - if isinstance(exc, BaseProcessingFail): - # Looks like yes, so record information about that failure and any - # metadata the user might have supplied. - entry[u'fail_error'] = exc.exception_path - entry[u'fail_metadata'] = exc.metadata - else: - # Looks like no, so just mark it as failed and don't record a - # failure_error (we'll assume it wasn't handled) and don't record - # metadata (in fact overwrite it if somehow it had previous info - # here) - entry[u'fail_error'] = None - entry[u'fail_metadata'] = {} - - entry.save() + entry_id = args[0] + mark_entry_failed(entry_id, exc) process_media = registry.tasks[ProcessMedia.name] +def mark_entry_failed(entry_id, exc): + # Was this a BaseProcessingFail? In other words, was this a + # type of error that we know how to handle? + if isinstance(exc, BaseProcessingFail): + # Looks like yes, so record information about that failure and any + # metadata the user might have supplied. + mgg.database['media_entries'].update( + {'_id': entry_id}, + {'$set': {u'state': u'failed', + u'fail_error': exc.exception_path, + u'fail_metadata': exc.metadata}}) + else: + # Looks like no, so just mark it as failed and don't record a + # failure_error (we'll assume it wasn't handled) and don't record + # metadata (in fact overwrite it if somehow it had previous info + # here) + mgg.database['media_entries'].update( + {'_id': entry_id}, + {'$set': {u'state': u'failed', + u'fail_error': None, + u'fail_metadata': {}}}) + + def process_image(entry): """ Code to process an image diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index 25b3664b..1ba17954 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -28,7 +28,7 @@ from mediagoblin.util import ( from mediagoblin.util import pass_to_ugettext as _ from mediagoblin.decorators import require_active_login from mediagoblin.submit import forms as submit_forms, security -from mediagoblin.process_media import process_media +from mediagoblin.process_media import process_media, mark_entry_failed from mediagoblin.messages import add_message, SUCCESS @@ -102,9 +102,22 @@ def submit_start(request): # # (... don't change entry after this point to avoid race # conditions with changes to the document via processing code) - process_media.apply_async( - [unicode(entry['_id'])], {}, - task_id=task_id) + try: + process_media.apply_async( + [unicode(entry['_id'])], {}, + task_id=task_id) + except BaseException as exc: + # The purpose of this section is because when running in "lazy" + # or always-eager-with-exceptions-propagated celery mode that + # the failure handling won't happen on Celery end. Since we + # expect a lot of users to run things in this way we have to + # capture stuff here. + # + # ... not completely the diaper pattern because the exception is + # re-raised :) + mark_entry_failed(entry[u'_id'], exc) + # re-raise the exception + raise add_message(request, SUCCESS, _('Woohoo! Submitted!')) From 2e5ea6b9b79244a0436264c5f1b606ec5328b70e Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 13 Aug 2011 12:52:22 -0500 Subject: [PATCH 09/14] @task decorator no longer used! Removing that import. --- mediagoblin/process_media/__init__.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index 69e4fc45..18836919 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -17,7 +17,7 @@ import Image from contextlib import contextmanager -from celery.task import task, Task +from celery.task import Task from celery import registry from mediagoblin.db.util import ObjectId @@ -86,6 +86,18 @@ process_media = registry.tasks[ProcessMedia.name] def mark_entry_failed(entry_id, exc): + """ + Mark a media entry as having failed in its conversion. + + Uses the exception that was raised to mark more information. If the + exception is a derivative of BaseProcessingFail then we can store extra + information that can be useful for users telling them why their media failed + to process. + + Args: + - entry_id: The id of the media entry + + """ # Was this a BaseProcessingFail? In other words, was this a # type of error that we know how to handle? if isinstance(exc, BaseProcessingFail): From ff520ff53b7a17204c64beea28c9dbde13c5cf26 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sat, 13 Aug 2011 12:52:56 -0500 Subject: [PATCH 10/14] Converting multi-line-string-comment to a real comment. --- mediagoblin/process_media/__init__.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index 18836919..e1289a4c 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -147,11 +147,9 @@ def process_image(entry): with closing(thumb_file): thumb.save(thumb_file, "JPEG", quality=90) - """ - If the size of the original file exceeds the specified size of a `medium` - file, a `medium.jpg` files is created and later associated with the media - entry. - """ + # If the size of the original file exceeds the specified size of a `medium` + # file, a `medium.jpg` files is created and later associated with the media + # entry. medium = Image.open(queued_filename) medium_processed = False From e3e9b8fcc962621e39a56748a7d34793a39e6bc6 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 14 Aug 2011 07:53:24 -0500 Subject: [PATCH 11/14] Switch BaseProcessingFail.exception_path's separator from period to colon Also removing .generator_error_message() which doesn't make sense really... we need to get the message when we don't have an instance of the exception, and this method requires an instance. --- mediagoblin/process_media/errors.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/mediagoblin/process_media/errors.py b/mediagoblin/process_media/errors.py index f2ae87ff..f8ae9ab2 100644 --- a/mediagoblin/process_media/errors.py +++ b/mediagoblin/process_media/errors.py @@ -29,22 +29,12 @@ class BaseProcessingFail(Exception): @property def exception_path(self): - return u"%s.%s" % ( + return u"%s:%s" % ( self.__class__.__module__, self.__class__.__name__) def __init__(self, **metadata): self.metadata = metadata or {} - def generate_error_message(self): - """ - Generate an error to display to users in the panel. - - Uses this class's general_message, possibly interpolated - with any metadata in self.metadata['error_message_vars'], - if appropriate. - """ - return self.general_message % self.metadata.get('error_message_vars', {}) - class BadMediaFail(BaseProcessingFail): """ From 6ee9c719025f954bfc996f11b4a89219f635a17f Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 14 Aug 2011 07:55:08 -0500 Subject: [PATCH 12/14] Method to get the failure exception object for a MediaEntry, if appropriate. --- mediagoblin/db/models.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 982883d7..b6e52441 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -297,6 +297,13 @@ class MediaEntry(Document): def uploader(self): return self.db.User.find_one({'_id': self['uploader']}) + def get_fail_exception(self): + """ + Get the exception that's appropriate for this error + """ + if self['fail_error']: + return util.import_component(self['fail_error']) + class MediaComment(Document): """ From 01c75c7ebaeee707271773c37f111ff9075d3656 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 14 Aug 2011 07:56:10 -0500 Subject: [PATCH 13/14] Processing panel view Now you can view your failed and in-process media items! --- mediagoblin/static/css/base.css | 12 ++++ .../user_pages/processing_panel.html | 67 +++++++++++++++++++ mediagoblin/user_pages/routing.py | 6 +- mediagoblin/user_pages/views.py | 52 +++++++++++++- 4 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 mediagoblin/templates/mediagoblin/user_pages/processing_panel.html diff --git a/mediagoblin/static/css/base.css b/mediagoblin/static/css/base.css index 59c2f49d..a421bb4d 100644 --- a/mediagoblin/static/css/base.css +++ b/mediagoblin/static/css/base.css @@ -291,3 +291,15 @@ ul.mediaentry_tags li { margin: 0px 5px 0px 0px; padding: 0px; } + + +/* media processing panel */ + +table.media_panel { + width: 100%; +} + +table.media_panel th { + font-weight: bold; + padding-bottom: 4px; +} \ No newline at end of file diff --git a/mediagoblin/templates/mediagoblin/user_pages/processing_panel.html b/mediagoblin/templates/mediagoblin/user_pages/processing_panel.html new file mode 100644 index 00000000..abc7efd3 --- /dev/null +++ b/mediagoblin/templates/mediagoblin/user_pages/processing_panel.html @@ -0,0 +1,67 @@ +{# +# GNU MediaGoblin -- federated, autonomous media hosting +# Copyright (C) 2011 Free Software Foundation, Inc +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +#} +{% extends "mediagoblin/base.html" %} + +{% block mediagoblin_content %} + +

{% trans %}Media processing panel{% endtrans %}

+ +

+ {% trans %}You can track the state of media being processed for your gallery here.{% endtrans %} +

+ +

{% trans %}Media in-processing{% endtrans %}

+ +{% if processing_entries.count() %} + + + + + + + {% for media_entry in processing_entries %} + + + + + + {% endfor %} +
TitleWhen submittedStatus
{{ media_entry['title'] }}{{ media_entry['created'].strftime("%m-%d-%Y %I:%M %p") }}
+{% else %} +

{% trans %}No media in-processing{% endtrans %}

+{% endif %} + +{% if failed_entries.count() %} +

{% trans %}These uploads failed to process:{% endtrans %}

+ + + + + + + + {% for media_entry in failed_entries %} + + + + + + {% endfor %} +
TitleWhen submittedReason for failure
{{ media_entry['title'] }}{{ media_entry['created'].strftime("%m-%d-%Y %I:%M %p") }}{{ media_entry.get_fail_exception().general_message }}
+{% endif %} +{% endblock %} diff --git a/mediagoblin/user_pages/routing.py b/mediagoblin/user_pages/routing.py index 3be0617d..bf9f12ab 100644 --- a/mediagoblin/user_pages/routing.py +++ b/mediagoblin/user_pages/routing.py @@ -33,4 +33,8 @@ user_routes = [ controller="mediagoblin.user_pages.views:atom_feed"), Route('mediagoblin.user_pages.media_post_comment', '/{user}/m/{media}/comment/add/', - controller="mediagoblin.user_pages.views:media_post_comment")] + controller="mediagoblin.user_pages.views:media_post_comment"), + Route('mediagoblin.user_pages.processing_panel', + '/{user}/panel/', + controller="mediagoblin.user_pages.views:processing_panel"), + ] diff --git a/mediagoblin/user_pages/views.py b/mediagoblin/user_pages/views.py index fb72a421..d4ff1fce 100644 --- a/mediagoblin/user_pages/views.py +++ b/mediagoblin/user_pages/views.py @@ -1,4 +1,4 @@ -# GNU MediaGoblin -- federated, autonomous media hosting +# MediaGoblin -- federated, autonomous media hosting # Copyright (C) 2011 Free Software Foundation, Inc # # This program is free software: you can redistribute it and/or modify @@ -175,3 +175,53 @@ def atom_feed(request): url=entry.url_for_self(request.urlgen)) return feed.get_response() + + +@require_active_login +def processing_panel(request): + """ + Show to the user what media is still in conversion/processing... + and what failed, and why! + """ + # Get the user + user = request.db.User.find_one( + {'username': request.matchdict['user'], + 'status': 'active'}) + + # Make sure the user exists and is active + if not user: + return exc.HTTPNotFound() + elif user['status'] != u'active': + return render_to_response( + request, + 'mediagoblin/user_pages/user.html', + {'user': user}) + + # XXX: Should this be a decorator? + # + # Make sure we have permission to access this user's panel. Only + # admins and this user herself should be able to do so. + if not (user[u'_id'] == request.user[u'_id'] + or request.user.is_admin): + # No? Let's simply redirect to this user's homepage then. + return redirect( + request, 'mediagoblin.user_pages.user_home', + user=request.matchdict['user']) + + # Get media entries which are in-processing + processing_entries = request.db.MediaEntry.find( + {'uploader': user['_id'], + 'state': 'processing'}).sort('created', DESCENDING) + + # Get media entries which have failed to process + failed_entries = request.db.MediaEntry.find( + {'uploader': user['_id'], + 'state': 'failed'}).sort('created', DESCENDING) + + # Render to response + return render_to_response( + request, + 'mediagoblin/user_pages/processing_panel.html', + {'user': user, + 'processing_entries': processing_entries, + 'failed_entries': failed_entries}) From 68f3ffbe8272c58b0421ad57f49374723838f0e4 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 14 Aug 2011 09:12:43 -0500 Subject: [PATCH 14/14] Malicious uploads test with fake but not really image files working! :) --- mediagoblin/tests/test_submission.py | 59 +++++++++++++++++----------- 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/mediagoblin/tests/test_submission.py b/mediagoblin/tests/test_submission.py index a7248255..9ae129cd 100644 --- a/mediagoblin/tests/test_submission.py +++ b/mediagoblin/tests/test_submission.py @@ -156,7 +156,7 @@ class TestSubmission: util.clear_test_template_context() response = self.test_app.post( '/submit/', { - 'title': 'Malicious Upload 2' + 'title': 'Malicious Upload 1' }, upload_files=[( 'file', EVIL_FILE)]) @@ -164,33 +164,46 @@ class TestSubmission: form = context['submit_form'] assert form.file.errors == ['The file doesn\'t seem to be an image!'] - # NOTE: The following 2 tests will fail. These can be uncommented - # after http://bugs.foocorp.net/issues/324 is resolved and - # bad files are handled properly. + # NOTE: The following 2 tests will ultimately fail, but they + # *will* pass the initial form submission step. Instead, + # they'll be caught as failures during the processing step. # Test non-supported file with .jpg extension # ------------------------------------------- - #util.clear_test_template_context() - #response = self.test_app.post( - # '/submit/', { - # 'title': 'Malicious Upload 2' - # }, upload_files=[( - # 'file', EVIL_JPG)]) + util.clear_test_template_context() + response = self.test_app.post( + '/submit/', { + 'title': 'Malicious Upload 2' + }, upload_files=[( + 'file', EVIL_JPG)]) + response.follow() + assert_equal( + urlparse.urlsplit(response.location)[2], + '/u/chris/') - #context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/submit/start.html'] - #form = context['submit_form'] - #assert form.file.errors == ['The file doesn\'t seem to be an image!'] + entry = mg_globals.database.MediaEntry.find_one( + {'title': 'Malicious Upload 2'}) + assert_equal(entry['state'], 'failed') + assert_equal( + entry['fail_error'], + u'mediagoblin.process_media.errors:BadMediaFail') # Test non-supported file with .png extension # ------------------------------------------- - #util.clear_test_template_context() - #response = self.test_app.post( - # '/submit/', { - # 'title': 'Malicious Upload 3' - # }, upload_files=[( - # 'file', EVIL_PNG)]) - - #context = util.TEMPLATE_TEST_CONTEXT['mediagoblin/submit/start.html'] - #form = context['submit_form'] - #assert form.file.errors == ['The file doesn\'t seem to be an image!'] + util.clear_test_template_context() + response = self.test_app.post( + '/submit/', { + 'title': 'Malicious Upload 3' + }, upload_files=[( + 'file', EVIL_PNG)]) + response.follow() + assert_equal( + urlparse.urlsplit(response.location)[2], + '/u/chris/') + entry = mg_globals.database.MediaEntry.find_one( + {'title': 'Malicious Upload 3'}) + assert_equal(entry['state'], 'failed') + assert_equal( + entry['fail_error'], + u'mediagoblin.process_media.errors:BadMediaFail')