From 180bdbde93ad4b62395c78e99e97587b44ad31c7 Mon Sep 17 00:00:00 2001 From: Elrond Date: Wed, 8 Jun 2011 23:22:11 +0200 Subject: [PATCH 01/15] Refactor filename generation in the public store Just a small refactoring of the filename setup in the public store. Very simple. --- mediagoblin/process_media/__init__.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index 4f06a686..097b4375 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -24,6 +24,13 @@ from mediagoblin.globals import database, queue_store, public_store THUMB_SIZE = 200, 200 +def create_pub_filepath(entry, filename): + return public_store.get_unique_filepath( + ['media_entries', + unicode(entry['_id']), + filename]) + + @task def process_media_initial(media_id): entry = database.MediaEntry.one( @@ -36,10 +43,7 @@ def process_media_initial(media_id): thumb = Image.open(queued_file) thumb.thumbnail(THUMB_SIZE, Image.ANTIALIAS) - thumb_filepath = public_store.get_unique_filepath( - ['media_entries', - unicode(entry['_id']), - 'thumbnail.jpg']) + thumb_filepath = create_pub_filepath(entry, 'thumbnail.jpg') with public_store.get_file(thumb_filepath, 'w') as thumb_file: thumb.save(thumb_file, "JPEG") @@ -49,15 +53,13 @@ def process_media_initial(media_id): queued_file = queue_store.get_file(queued_filepath, 'rb') with queued_file: - main_filepath = public_store.get_unique_filepath( - ['media_entries', - unicode(entry['_id']), - queued_filepath[-1]]) + main_filepath = create_pub_filepath(entry, queued_filepath[-1]) with public_store.get_file(main_filepath, 'wb') as main_file: main_file.write(queued_file.read()) queue_store.delete_file(queued_filepath) + entry['queued_media_file'] = [] media_files_dict = entry.setdefault('media_files', {}) media_files_dict['thumb'] = thumb_filepath media_files_dict['main'] = main_filepath From e893094a06ab5d659b82c2d2fe646b1545c131dd Mon Sep 17 00:00:00 2001 From: Elrond Date: Fri, 10 Jun 2011 21:20:18 +0200 Subject: [PATCH 02/15] celery_setup: drop param to setup_self and simplify OUR_MODULENAME setup_self used to look like this: setup_self(setup_globals_func=setup_globals) The function isn't called with any param, so drop it. Rewrite function as needed. The module var OUR_MODULENAME just has the module's name in it. This is available as __name__ anyway, so use this to initialize the var. --- mediagoblin/celery_setup/from_celery.py | 7 +++---- mediagoblin/celery_setup/from_tests.py | 5 ++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/mediagoblin/celery_setup/from_celery.py b/mediagoblin/celery_setup/from_celery.py index 0669e80c..6fab59ca 100644 --- a/mediagoblin/celery_setup/from_celery.py +++ b/mediagoblin/celery_setup/from_celery.py @@ -23,13 +23,12 @@ from mediagoblin import storage from mediagoblin.db.open import setup_connection_and_db_from_config from mediagoblin.celery_setup import setup_celery_from_config from mediagoblin.globals import setup_globals -from mediagoblin import globals as mgoblin_globals -OUR_MODULENAME = 'mediagoblin.celery_setup.from_celery' +OUR_MODULENAME = __name__ -def setup_self(setup_globals_func=setup_globals): +def setup_self(): """ Transform this module into a celery config module by reading the mediagoblin config file. Set the environment variable @@ -76,7 +75,7 @@ def setup_self(setup_globals_func=setup_globals): queue_store = storage.storage_system_from_paste_config( mgoblin_section, 'queuestore') - setup_globals_func( + setup_globals( db_connection=connection, database=db, public_store=public_store, diff --git a/mediagoblin/celery_setup/from_tests.py b/mediagoblin/celery_setup/from_tests.py index fe7d7314..70814075 100644 --- a/mediagoblin/celery_setup/from_tests.py +++ b/mediagoblin/celery_setup/from_tests.py @@ -19,13 +19,12 @@ import os from mediagoblin.tests.tools import TEST_APP_CONFIG from mediagoblin import util from mediagoblin.celery_setup import setup_celery_from_config -from mediagoblin.globals import setup_globals -OUR_MODULENAME = 'mediagoblin.celery_setup.from_tests' +OUR_MODULENAME = __name__ -def setup_self(setup_globals_func=setup_globals): +def setup_self(): """ Set up celery for testing's sake, which just needs to set up celery and celery only. From 12c559447a75fa4b43872ccda544762a331a1bed Mon Sep 17 00:00:00 2001 From: Elrond Date: Fri, 10 Jun 2011 21:59:04 +0200 Subject: [PATCH 03/15] Tests: Kill the whole testing database after all tests nose allows setup and teardown functions at the package level. So use this to drop the complete database after all tests have finished. --- mediagoblin/tests/__init__.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/mediagoblin/tests/__init__.py b/mediagoblin/tests/__init__.py index c129cbf8..46c7fd69 100644 --- a/mediagoblin/tests/__init__.py +++ b/mediagoblin/tests/__init__.py @@ -13,3 +13,14 @@ # # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . + +from .. import globals + + +def setup_package(): + pass + +def teardown_package(): + print "Killing db ..." + globals.db_connection.drop_database(globals.database.name) + print "... done" From 6e7ce8d1af8c6fcf7d00992b1c8ef0e8c1602479 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 12 Jun 2011 17:27:37 -0500 Subject: [PATCH 04/15] mediagoblin.globals->mediagoblin.mg_globals --- mediagoblin/app.py | 2 +- mediagoblin/auth/lib.py | 4 ++-- mediagoblin/celery_setup/from_celery.py | 2 +- mediagoblin/db/migrations.py | 2 -- mediagoblin/db/models.py | 4 ++-- mediagoblin/gmg_commands/migrate.py | 1 - mediagoblin/gmg_commands/shell.py | 8 ++++---- mediagoblin/{globals.py => mg_globals.py} | 2 +- mediagoblin/process_media/__init__.py | 2 +- mediagoblin/tests/__init__.py | 4 ++-- mediagoblin/tests/test_auth.py | 10 +++++----- mediagoblin/tests/test_globals.py | 2 +- mediagoblin/tests/test_tests.py | 10 +++++----- mediagoblin/util.py | 14 +++++++------- 14 files changed, 32 insertions(+), 35 deletions(-) rename mediagoblin/{globals.py => mg_globals.py} (92%) diff --git a/mediagoblin/app.py b/mediagoblin/app.py index 5d594039..a1c6b512 100644 --- a/mediagoblin/app.py +++ b/mediagoblin/app.py @@ -23,7 +23,7 @@ from webob import Request, exc from mediagoblin import routing, util, storage, staticdirect from mediagoblin.db.open import setup_connection_and_db_from_config -from mediagoblin.globals import setup_globals +from mediagoblin.mg_globals import setup_globals from mediagoblin.celery_setup import setup_celery_from_config from mediagoblin.workbench import WorkbenchManager, DEFAULT_WORKBENCH_DIR diff --git a/mediagoblin/auth/lib.py b/mediagoblin/auth/lib.py index f40e560f..08bbdd16 100644 --- a/mediagoblin/auth/lib.py +++ b/mediagoblin/auth/lib.py @@ -20,7 +20,7 @@ import random import bcrypt from mediagoblin.util import send_email, render_template -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals def bcrypt_check_password(raw_pass, stored_hash, extra_salt=None): @@ -112,7 +112,7 @@ def send_verification_email(user, request): # TODO: There is no error handling in place send_email( - mgoblin_globals.email_sender_address, + mg_globals.email_sender_address, [user['email']], # TODO # Due to the distributed nature of GNU MediaGoblin, we should diff --git a/mediagoblin/celery_setup/from_celery.py b/mediagoblin/celery_setup/from_celery.py index 71f93d76..c8ccebc8 100644 --- a/mediagoblin/celery_setup/from_celery.py +++ b/mediagoblin/celery_setup/from_celery.py @@ -22,7 +22,7 @@ from paste.deploy.converters import asbool from mediagoblin import storage from mediagoblin.db.open import setup_connection_and_db_from_config from mediagoblin.celery_setup import setup_celery_from_config -from mediagoblin.globals import setup_globals +from mediagoblin.mg_globals import setup_globals from mediagoblin.workbench import WorkbenchManager, DEFAULT_WORKBENCH_DIR diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index d035b15b..f1f625b7 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -16,8 +16,6 @@ from mongokit import DocumentMigration -from mediagoblin import globals as mediagoblin_globals - class MediaEntryMigration(DocumentMigration): def allmigration01_uploader_to_reference(self): diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 3da97a49..d77cf619 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -20,7 +20,7 @@ from mongokit import Document, Set from mediagoblin import util from mediagoblin.auth import lib as auth_lib -from mediagoblin import globals as mediagoblin_globals +from mediagoblin import mg_globals from mediagoblin.db import migrations from mediagoblin.db.util import ObjectId @@ -114,7 +114,7 @@ class MediaEntry(Document): def generate_slug(self): self['slug'] = util.slugify(self['title']) - duplicate = mediagoblin_globals.database.media_entries.find_one( + duplicate = mg_globals.database.media_entries.find_one( {'slug': self['slug']}) if duplicate: diff --git a/mediagoblin/gmg_commands/migrate.py b/mediagoblin/gmg_commands/migrate.py index e04fb343..3ce25701 100644 --- a/mediagoblin/gmg_commands/migrate.py +++ b/mediagoblin/gmg_commands/migrate.py @@ -17,7 +17,6 @@ from mediagoblin.db import migrations from mediagoblin.gmg_commands import util as commands_util -from mediagoblin import globals as mgoblin_globals def migrate_parser_setup(subparser): diff --git a/mediagoblin/gmg_commands/shell.py b/mediagoblin/gmg_commands/shell.py index 9c0259de..16caf398 100644 --- a/mediagoblin/gmg_commands/shell.py +++ b/mediagoblin/gmg_commands/shell.py @@ -17,7 +17,7 @@ import code -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals from mediagoblin.gmg_commands import util as commands_util @@ -35,7 +35,7 @@ GNU MediaGoblin shell! ---------------------- Available vars: - mgoblin_app: instantiated mediagoblin application - - mgoblin_globals: mediagoblin.globals + - mg_globals: mediagoblin.globals - db: database instance """ @@ -50,5 +50,5 @@ def shell(args): banner=SHELL_BANNER, local={ 'mgoblin_app': mgoblin_app, - 'mgoblin_globals': mgoblin_globals, - 'db': mgoblin_globals.database}) + 'mg_globals': mg_globals, + 'db': mg_globals.database}) diff --git a/mediagoblin/globals.py b/mediagoblin/mg_globals.py similarity index 92% rename from mediagoblin/globals.py rename to mediagoblin/mg_globals.py index 80d1f01d..2fca3c0a 100644 --- a/mediagoblin/globals.py +++ b/mediagoblin/mg_globals.py @@ -27,7 +27,7 @@ translations = gettext.find( def setup_globals(**kwargs): - from mediagoblin import globals as mg_globals + from mediagoblin import mg_globals for key, value in kwargs.iteritems(): setattr(mg_globals, key, value) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index 531eb16d..71a0a277 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -18,7 +18,7 @@ import Image from mediagoblin.db.util import ObjectId from celery.task import task -from mediagoblin import globals as mg_globals +from mediagoblin import mg_globals THUMB_SIZE = 200, 200 diff --git a/mediagoblin/tests/__init__.py b/mediagoblin/tests/__init__.py index 46c7fd69..e9e2a59a 100644 --- a/mediagoblin/tests/__init__.py +++ b/mediagoblin/tests/__init__.py @@ -14,7 +14,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -from .. import globals +from mediagoblin import mg_globals def setup_package(): @@ -22,5 +22,5 @@ def setup_package(): def teardown_package(): print "Killing db ..." - globals.db_connection.drop_database(globals.database.name) + mg_globals.db_connection.drop_database(mg_globals.database.name) print "... done" diff --git a/mediagoblin/tests/test_auth.py b/mediagoblin/tests/test_auth.py index cdfeccab..3d569093 100644 --- a/mediagoblin/tests/test_auth.py +++ b/mediagoblin/tests/test_auth.py @@ -20,7 +20,7 @@ from nose.tools import assert_equal from mediagoblin.auth import lib as auth_lib from mediagoblin.tests.tools import setup_fresh_app -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals from mediagoblin import util @@ -137,7 +137,7 @@ def test_register_views(test_app): u'Passwords must match.'] ## At this point there should be no users in the database ;) - assert not mgoblin_globals.database.User.find().count() + assert not mg_globals.database.User.find().count() # Successful register # ------------------- @@ -158,7 +158,7 @@ def test_register_views(test_app): 'mediagoblin/auth/register_success.html') ## Make sure user is in place - new_user = mgoblin_globals.database.User.find_one( + new_user = mg_globals.database.User.find_one( {'username': 'happygirl'}) assert new_user assert new_user['status'] == u'needs_email_verification' @@ -191,7 +191,7 @@ def test_register_views(test_app): context = util.TEMPLATE_TEST_CONTEXT[ 'mediagoblin/auth/verify_email.html'] assert context['verification_successful'] == False - new_user = mgoblin_globals.database.User.find_one( + new_user = mg_globals.database.User.find_one( {'username': 'happygirl'}) assert new_user assert new_user['status'] == u'needs_email_verification' @@ -203,7 +203,7 @@ def test_register_views(test_app): context = util.TEMPLATE_TEST_CONTEXT[ 'mediagoblin/auth/verify_email.html'] assert context['verification_successful'] == True - new_user = mgoblin_globals.database.User.find_one( + new_user = mg_globals.database.User.find_one( {'username': 'happygirl'}) assert new_user assert new_user['status'] == u'active' diff --git a/mediagoblin/tests/test_globals.py b/mediagoblin/tests/test_globals.py index 6d2e01da..59d217f3 100644 --- a/mediagoblin/tests/test_globals.py +++ b/mediagoblin/tests/test_globals.py @@ -14,7 +14,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -from mediagoblin import globals as mg_globals +from mediagoblin import mg_globals def test_setup_globals(): mg_globals.setup_globals( diff --git a/mediagoblin/tests/test_tests.py b/mediagoblin/tests/test_tests.py index 3ecbfac7..8ac7f0a4 100644 --- a/mediagoblin/tests/test_tests.py +++ b/mediagoblin/tests/test_tests.py @@ -16,7 +16,7 @@ from mediagoblin.tests.tools import get_test_app -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals def test_get_test_app_wipes_db(): @@ -24,15 +24,15 @@ def test_get_test_app_wipes_db(): Make sure we get a fresh database on every wipe :) """ get_test_app() - assert mgoblin_globals.database.User.find().count() == 0 + assert mg_globals.database.User.find().count() == 0 - new_user = mgoblin_globals.database.User() + new_user = mg_globals.database.User() new_user['username'] = u'lolcat' new_user['email'] = u'lol@cats.example.org' new_user['pw_hash'] = u'pretend_this_is_a_hash' new_user.save() - assert mgoblin_globals.database.User.find().count() == 1 + assert mg_globals.database.User.find().count() == 1 get_test_app() - assert mgoblin_globals.database.User.find().count() == 0 + assert mg_globals.database.User.find().count() == 0 diff --git a/mediagoblin/util.py b/mediagoblin/util.py index 64e21ca9..f29f8570 100644 --- a/mediagoblin/util.py +++ b/mediagoblin/util.py @@ -31,7 +31,7 @@ import translitcodec from paste.deploy.loadwsgi import NicerConfigParser from webob import Response, exc -from mediagoblin import globals as mgoblin_globals +from mediagoblin import mg_globals from mediagoblin.db.util import ObjectId @@ -102,8 +102,8 @@ def get_jinja_env(template_loader, locale): extensions=['jinja2.ext.i18n']) template_env.install_gettext_callables( - mgoblin_globals.translations.gettext, - mgoblin_globals.translations.ngettext) + mg_globals.translations.gettext, + mg_globals.translations.ngettext) if exists(locale): SETUP_JINJA_ENVS[locale] = template_env @@ -264,9 +264,9 @@ def send_email(from_addr, to_addrs, subject, message_body): - message_body: email body text """ # TODO: make a mock mhost if testing is enabled - if TESTS_ENABLED or mgoblin_globals.email_debug_mode: + if TESTS_ENABLED or mg_globals.email_debug_mode: mhost = FakeMhost() - elif not mgoblin_globals.email_debug_mode: + elif not mg_globals.email_debug_mode: mhost = smtplib.SMTP() mhost.connect() @@ -279,7 +279,7 @@ def send_email(from_addr, to_addrs, subject, message_body): if TESTS_ENABLED: EMAIL_TEST_INBOX.append(message) - if getattr(mgoblin_globals, 'email_debug_mode', False): + if getattr(mg_globals, 'email_debug_mode', False): print u"===== Email =====" print u"From address: %s" % message['From'] print u"To addresses: %s" % message['To'] @@ -393,7 +393,7 @@ def setup_gettext(locale): if exists(locale): SETUP_GETTEXTS[locale] = this_gettext - mgoblin_globals.setup_globals( + mg_globals.setup_globals( translations=this_gettext) From 300c34e8ce52f20ba789c22c26bdc78b9ecb2954 Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Sun, 12 Jun 2011 17:28:54 -0500 Subject: [PATCH 05/15] First import of mg_globals as mgg, partly because I just wanted it to be clear that it's okay to do by doing it *somewhere* :) --- mediagoblin/process_media/__init__.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index 71a0a277..c71c5dda 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -18,7 +18,7 @@ import Image from mediagoblin.db.util import ObjectId from celery.task import task -from mediagoblin import mg_globals +from mediagoblin import mg_globals as mgg THUMB_SIZE = 200, 200 @@ -26,14 +26,14 @@ THUMB_SIZE = 200, 200 @task def process_media_initial(media_id): - workbench = mg_globals.workbench_manager.create_workbench() + workbench = mgg.workbench_manager.create_workbench() - entry = mg_globals.database.MediaEntry.one( + entry = mgg.database.MediaEntry.one( {'_id': ObjectId(media_id)}) queued_filepath = entry['queued_media_file'] - queued_filename = mg_globals.workbench_manager.localized_file( - workbench, mg_globals.queue_store, queued_filepath, + queued_filename = mgg.workbench_manager.localized_file( + workbench, mgg.queue_store, queued_filepath, 'source') queued_file = file(queued_filename, 'r') @@ -42,12 +42,12 @@ def process_media_initial(media_id): thumb = Image.open(queued_file) thumb.thumbnail(THUMB_SIZE, Image.ANTIALIAS) - thumb_filepath = mg_globals.public_store.get_unique_filepath( + thumb_filepath = mgg.public_store.get_unique_filepath( ['media_entries', unicode(entry['_id']), 'thumbnail.jpg']) - thumb_file = mg_globals.public_store.get_file(thumb_filepath, 'w') + thumb_file = mgg.public_store.get_file(thumb_filepath, 'w') with thumb_file: thumb.save(thumb_file, "JPEG") @@ -56,15 +56,15 @@ def process_media_initial(media_id): queued_file = file(queued_filename, 'rb') with queued_file: - main_filepath = mg_globals.public_store.get_unique_filepath( + main_filepath = mgg.public_store.get_unique_filepath( ['media_entries', unicode(entry['_id']), queued_filepath[-1]]) - with mg_globals.public_store.get_file(main_filepath, 'wb') as main_file: + with mgg.public_store.get_file(main_filepath, 'wb') as main_file: main_file.write(queued_file.read()) - mg_globals.queue_store.delete_file(queued_filepath) + mgg.queue_store.delete_file(queued_filepath) media_files_dict = entry.setdefault('media_files', {}) media_files_dict['thumb'] = thumb_filepath media_files_dict['main'] = main_filepath @@ -72,4 +72,4 @@ def process_media_initial(media_id): entry.save() # clean up workbench - mg_globals.workbench_manager.destroy_workbench(workbench) + mgg.workbench_manager.destroy_workbench(workbench) From 34d35a23930815438a4e1d8b28ec17016fb938c0 Mon Sep 17 00:00:00 2001 From: cfdv Date: Sun, 12 Jun 2011 17:35:07 -0500 Subject: [PATCH 06/15] ensure color mode compatibility when making image thumbnails --- mediagoblin/process_media/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index c71c5dda..f0a6e511 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -41,6 +41,9 @@ def process_media_initial(media_id): with queued_file: thumb = Image.open(queued_file) thumb.thumbnail(THUMB_SIZE, Image.ANTIALIAS) + # ensure color mode is compatible with jpg + if thumb.mode != "RGB": + thumb = thumb.convert("RGB") thumb_filepath = mgg.public_store.get_unique_filepath( ['media_entries', From 52426ae01f0439af2833656a74eda70a240d66ae Mon Sep 17 00:00:00 2001 From: Elrond Date: Mon, 13 Jun 2011 00:36:56 +0200 Subject: [PATCH 07/15] Create a Workbench class and use it everywhere. Some references to Workbench.dir look ugly, I'm happy to hear suggestions there. --- mediagoblin/process_media/__init__.py | 4 +- mediagoblin/tests/test_workbench.py | 30 ++++++---- mediagoblin/workbench.py | 83 +++++++++++++++++---------- 3 files changed, 72 insertions(+), 45 deletions(-) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index f0a6e511..bd067e39 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -32,8 +32,8 @@ def process_media_initial(media_id): {'_id': ObjectId(media_id)}) queued_filepath = entry['queued_media_file'] - queued_filename = mgg.workbench_manager.localized_file( - workbench, mgg.queue_store, queued_filepath, + queued_filename = workbench.localized_file( + mgg.queue_store, queued_filepath, 'source') queued_file = file(queued_filename, 'r') diff --git a/mediagoblin/tests/test_workbench.py b/mediagoblin/tests/test_workbench.py index 89f2ef33..2795cd63 100644 --- a/mediagoblin/tests/test_workbench.py +++ b/mediagoblin/tests/test_workbench.py @@ -30,24 +30,31 @@ class TestWorkbench(object): def test_create_workbench(self): workbench = self.workbench_manager.create_workbench() - assert os.path.isdir(workbench) - assert workbench.startswith(self.workbench_manager.base_workbench_dir) + assert os.path.isdir(workbench.dir) + assert workbench.dir.startswith(self.workbench_manager.base_workbench_dir) + + def test_joinpath(self): + this_workbench = self.workbench_manager.create_workbench() + tmpname = this_workbench.joinpath('temp.txt') + assert tmpname == os.path.join(this_workbench.dir, 'temp.txt') + self.workbench_manager.destroy_workbench(this_workbench) def test_destroy_workbench(self): # kill a workbench this_workbench = self.workbench_manager.create_workbench() - tmpfile = file(os.path.join(this_workbench, 'temp.txt'), 'w') + tmpfile_name = this_workbench.joinpath('temp.txt') + tmpfile = file(tmpfile_name, 'w') with tmpfile: tmpfile.write('lollerskates') - assert os.path.exists(os.path.join(this_workbench, 'temp.txt')) + assert os.path.exists(tmpfile_name) self.workbench_manager.destroy_workbench(this_workbench) - assert not os.path.exists(os.path.join(this_workbench, 'temp.txt')) - assert not os.path.exists(this_workbench) + assert not os.path.exists(tmpfile_name) + assert not os.path.exists(this_workbench.dir) # make sure we can't kill other stuff though - dont_kill_this = tempfile.mkdtemp() + dont_kill_this = workbench.Workbench(tempfile.mkdtemp()) assert_raises( workbench.WorkbenchOutsideScope, @@ -65,8 +72,7 @@ class TestWorkbench(object): our_file.write('Our file') # with a local file storage - filename = self.workbench_manager.localized_file( - this_workbench, this_storage, filepath) + filename = this_workbench.localized_file(this_storage, filepath) assert filename == os.path.join( tmpdir, 'dir1/dir2/ourfile.txt') @@ -80,17 +86,17 @@ class TestWorkbench(object): filename = self.workbench_manager.localized_file( this_workbench, this_storage, filepath) assert filename == os.path.join( - this_workbench, 'ourfile.txt') + this_workbench.dir, 'ourfile.txt') # fake remote file storage, filename_if_copying set filename = self.workbench_manager.localized_file( this_workbench, this_storage, filepath, 'thisfile') assert filename == os.path.join( - this_workbench, 'thisfile.txt') + this_workbench.dir, 'thisfile.txt') # fake remote file storage, filename_if_copying set, # keep_extension_if_copying set to false filename = self.workbench_manager.localized_file( this_workbench, this_storage, filepath, 'thisfile.text', False) assert filename == os.path.join( - this_workbench, 'thisfile.text') + this_workbench.dir, 'thisfile.text') diff --git a/mediagoblin/workbench.py b/mediagoblin/workbench.py index d7252623..c88b686c 100644 --- a/mediagoblin/workbench.py +++ b/mediagoblin/workbench.py @@ -36,41 +36,24 @@ class WorkbenchOutsideScope(Exception): # Actual workbench stuff # ---------------------- -class WorkbenchManager(object): +class Workbench(object): """ - A system for generating and destroying workbenches. - - Workbenches are actually just subdirectories of a temporary storage space - for during the processing stage. + Represent the directory for the workbench """ + def __init__(self, dir): + self.dir = dir - def __init__(self, base_workbench_dir): - self.base_workbench_dir = os.path.abspath(base_workbench_dir) - if not os.path.exists(self.base_workbench_dir): - os.makedirs(self.base_workbench_dir) - - def create_workbench(self): - """ - Create and return the path to a new workbench (directory). - """ - return tempfile.mkdtemp(dir=self.base_workbench_dir) + def __unicode__(self): + return unicode(self.dir) + def __str__(self): + return str(self.dir) + def __repr__(self): + return repr(self.dir) - def destroy_workbench(self, workbench): - """ - Destroy this workbench! Deletes the directory and all its contents! + def joinpath(self, *args): + return os.path.join(self.dir, *args) - Makes sure the workbench actually belongs to this manager though. - """ - # just in case - workbench = os.path.abspath(workbench) - - if not workbench.startswith(self.base_workbench_dir): - raise WorkbenchOutsideScope( - "Can't destroy workbench outside the base workbench dir") - - shutil.rmtree(workbench) - - def localized_file(self, workbench, storage, filepath, + def localized_file(self, storage, filepath, filename_if_copying=None, keep_extension_if_copying=True): """ @@ -126,10 +109,48 @@ class WorkbenchManager(object): dest_filename = filename_if_copying full_dest_filename = os.path.join( - workbench, dest_filename) + self.dir, dest_filename) # copy it over storage.copy_locally( filepath, full_dest_filename) return full_dest_filename + + +class WorkbenchManager(object): + """ + A system for generating and destroying workbenches. + + Workbenches are actually just subdirectories of a temporary storage space + for during the processing stage. + """ + + def __init__(self, base_workbench_dir): + self.base_workbench_dir = os.path.abspath(base_workbench_dir) + if not os.path.exists(self.base_workbench_dir): + os.makedirs(self.base_workbench_dir) + + def create_workbench(self): + """ + Create and return the path to a new workbench (directory). + """ + return Workbench(tempfile.mkdtemp(dir=self.base_workbench_dir)) + + def destroy_workbench(self, workbench): + """ + Destroy this workbench! Deletes the directory and all its contents! + + Makes sure the workbench actually belongs to this manager though. + """ + # just in case + workbench = os.path.abspath(workbench.dir) + + if not workbench.startswith(self.base_workbench_dir): + raise WorkbenchOutsideScope( + "Can't destroy workbench outside the base workbench dir") + + shutil.rmtree(workbench) + + def localized_file(self, workbench, *args, **kwargs): + return workbench.localized_file(*args, **kwargs) From 909c639d07aee08e2a3cd039e6fe4ae397dce5a7 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Sun, 12 Jun 2011 22:42:10 -0400 Subject: [PATCH 08/15] Fixes git workflow * overhauls the docs so they're (hopefully) clearer on the git workflow * adds text about putting things in bugfix branches, documenting your work, and using the issue tracker * adds a contrived example that uses aliens --- docs/git.rst | 184 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 154 insertions(+), 30 deletions(-) diff --git a/docs/git.rst b/docs/git.rst index 0db1dacf..ddbcf226 100644 --- a/docs/git.rst +++ b/docs/git.rst @@ -2,11 +2,17 @@ Git, Cloning and Patches ========================== -GNU MediaGoblin uses git for all our version control and we have -the repositories hosted on `Gitorious `_. +GNU MediaGoblin uses git for all our version control and we have the +repositories hosted on `Gitorious `_. We have +two repositories: -We have two repositories. One is for the project and the other is for -the project website. +* MediaGoblin software: http://gitorious.org/mediagoblin/mediagoblin +* MediaGoblin website: http://gitorious.org/mediagoblin/mediagoblin-website + +It's most likely you want to look at the software repository--not the +website one. + +The rest of this chapter talks about using the software repository. How to clone the project @@ -20,46 +26,164 @@ Do:: How to send in patches ====================== +**Tie your changes to issues in the issue tracker** + All patches should be tied to issues in the `issue tracker -`_. -That makes it a lot easier for everyone to track proposed changes and -make sure your hard work doesn't get dropped on the floor! +`_. That makes +it a lot easier for everyone to track proposed changes and make sure +your hard work doesn't get dropped on the floor! If there isn't an +issue for what you're working on, please create one. The better the +description of what it is you're trying to fix/implement, the better +everyone else is able to understand why you're doing what you're +doing. -If there isn't an issue for what you're working on, please create -one. The better the description of what it is you're trying to -fix/implement, the better everyone else is able to understand why -you're doing what you're doing. +**Use bugfix branches** -There are two ways you could send in a patch. +The best way to isolate your changes is to create a branch based off +of the MediaGoblin repository master branch, do the changes related to +that one issue there, and then let us know how to get it. + +It's much easier on us if you isolate your changes to a branch focused +on the issue. Then we don't have to sift through things. + +It's much easier on you if you isolate your changes to a branch +focused on the issue. Then when we merge your changes in, you just +have to do a ``git fetch`` and that's it. This is especially true if +we reject some of your changes, but accept others or otherwise tweak +your changes. + +Further, if you isolate your changes to a branch, then you can work on +multiple issues at the same time and they don't conflict with one +another. + +**Properly document your changes** + +Include comments in the code. + +Write comprehensive commit messages. The better your commit message +is at describing what you did and why, the easier it is for us to +quickly accept your patch. + +Write comprehensive comments in the issue tracker about what you're +doing and why. -How to send in a patch from a publicly available clone ------------------------------------------------------- +**How to send us your changes** -Add a comment to the issue you're working on with the following bits -of information: +There are three ways to let us know how to get it: -* the url for your clone -* the revs you want looked at -* any details, questions, or other things that should be known +1. (preferred) **push changes to publicly available git clone and let + us know where to find it** + + Push your branch to your publicly available git clone and add a + comment to the issue with the url for your clone and the branch to + look at. + +2. **attaching the patch files to the issue** + + Run:: + + git format-patch -o patches /master + + Then tar up the newly created ``patches`` directory and attach the + directory to the issue. -How to send in a patch if you don't have a publicly available clone -------------------------------------------------------------------- +Example workflow +================ +Here's an example workflow. -Assuming that the remote is our repository on gitorious and the branch -to compare against is master, do the following: -1. checkout the branch you did your work in -2. do:: +Contributing changes +-------------------- - git format-patch -o patches origin/master +Slartibartfast from the planet Magrathea far off in the universe has +decided that he is bored with fjords and wants to fix issue 42 and +send us the changes. -3. either: +Slartibartfast has cloned the MediaGoblin repository and his clone +lives on gitorious. - * tar up and attach the tarball to the issue you're working on, OR - * attach the patch files to the issue you're working on one at a - time +Slartibartfast works locally. The remote named ``origin`` points to +his clone on gitorious. The remote named ``gmg`` points to the +MediaGoblin repository. + +Slartibartfast does the following: + +1. Fetches the latest from the MediaGoblin repository:: + + git fetch --all -p + +2. Creates a branch from the tip of the MediaGoblin repository (the + remote is named ``gmg``) master branch called ``issue_42``:: + + git checkout -b issue_42 gmg/master + +3. Slartibartfast works hard on his changes in the ``issue_42`` + branch. When done, he wants to notify us that he has made changes + he wants us to see. + +4. Slartibartfast pushes his changes to his clone (the remote is named + ``origin``):: + + git push origin issue_42 + +5. Slartibartfast adds a comment to issue 42 with the url for his + repository and the name of the branch he put the code in. He also + explains what he did and why it addresses the issue. + + +Updating a contribution +----------------------- + +Slartibartfast brushes his hands off with the sense of accomplishment +that comes with the knowledge of a job well done. He stands, wanders +over to get a cup of water, then realizes that he forgot to run the +unit tests! + +He runs the unit tests and discovers there's a bug in the code! + +Then he does this: + +1. He checks out the ``issue_42`` branch:: + + git checkout issue_42 + +2. He fixes the bug and checks it into the ``issue_42`` branch. + +3. He pushes his changes to his clone (the remote is named ``origin``):: + + git push origin issue_42 + +4. He adds another comment to issue 42 explaining about the mistake + and how he fixed it and that he's pushed the new change to the + ``issue_42`` branch of his publicly available clone. + + +What happens next +----------------- + +Slartibartfast is once again happy with his work. He finds issue 42 +in the issue tracker and adds a comment saying he submitted a merge +request with his changes and explains what they are. + +Later, someone checks out his code and finds a problem with it. He +adds a comment to the issue tracker specifying the problem and asks +Slartibartfast to fix it. Slartibartfst goes through the above steps +again, fixes the issue, pushes it to his ``issue_42`` branch and adds +another comment to the issue tracker about how he fixed it. + +Later, someone checks out his code and is happy with it. Someone +pulls it into the master branch of the MediaGoblin repository and adds +another comment to the issue and probably closes the issue out. + +Slartibartfast is notified of this. Slartibartfast does a:: + + git fetch --all + +The changes show up in the ``master`` branch of the ``gmg`` remote. +Slartibartfast now deletes his ``issue_42`` branch because he doesn't +need it anymore. How to learn git From fb6924170d0cc6b8648627edebc794e82b0946d7 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Sun, 12 Jun 2011 22:46:25 -0400 Subject: [PATCH 09/15] Tweaks git workflow structure * minor tweaking of the headers of the git workflow to break things up and organize them a bit better --- docs/git.rst | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/docs/git.rst b/docs/git.rst index ddbcf226..9a44867a 100644 --- a/docs/git.rst +++ b/docs/git.rst @@ -23,10 +23,11 @@ Do:: git clone git://gitorious.org/mediagoblin/mediagoblin.git -How to send in patches -====================== +How to contribute changes +========================= -**Tie your changes to issues in the issue tracker** +Tie your changes to issues in the issue tracker +----------------------------------------------- All patches should be tied to issues in the `issue tracker `_. That makes @@ -37,7 +38,9 @@ description of what it is you're trying to fix/implement, the better everyone else is able to understand why you're doing what you're doing. -**Use bugfix branches** + +Use bugfix branches to make changes +----------------------------------- The best way to isolate your changes is to create a branch based off of the MediaGoblin repository master branch, do the changes related to @@ -56,7 +59,9 @@ Further, if you isolate your changes to a branch, then you can work on multiple issues at the same time and they don't conflict with one another. -**Properly document your changes** + +Properly document your changes +------------------------------ Include comments in the code. @@ -68,7 +73,8 @@ Write comprehensive comments in the issue tracker about what you're doing and why. -**How to send us your changes** +How to send us your changes +--------------------------- There are three ways to let us know how to get it: From 1e85d28e018e6dc0d4be9af82f221ac5450423bb Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 13 Jun 2011 09:08:18 -0500 Subject: [PATCH 10/15] Already mentioned, but clarifying that branches should be localized to a feature/bugfix/issue. --- docs/git.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/git.rst b/docs/git.rst index 9a44867a..6421e093 100644 --- a/docs/git.rst +++ b/docs/git.rst @@ -81,9 +81,9 @@ There are three ways to let us know how to get it: 1. (preferred) **push changes to publicly available git clone and let us know where to find it** - Push your branch to your publicly available git clone and add a - comment to the issue with the url for your clone and the branch to - look at. + Push your feature/bugfix/issue branch to your publicly available + git clone and add a comment to the issue with the url for your + clone and the branch to look at. 2. **attaching the patch files to the issue** From 03db7d6c7416010a8eadecf93037ea398e6e07db Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 13 Jun 2011 12:24:52 -0400 Subject: [PATCH 11/15] Updates version in docs --- docs/conf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index fedaf33c..0e75a617 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -48,9 +48,9 @@ copyright = u'2011, Free Software Foundation, Inc and contributors' # built documents. # # The short X.Y version. -version = '0.0.1' +version = '0.0.2' # The full version, including alpha/beta/rc tags. -release = '0.0.1' +release = '0.0.2' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. From 6729d65a3246208b658a3780c7fd62e20c666aa0 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 13 Jun 2011 12:31:23 -0400 Subject: [PATCH 12/15] Adds local toc sections * Some of our chapters are pretty long and this should make it much easier for a user to find what they're looking for and jumping to it. It's easier to read the section toc at the top of the chapter, than it is to read it in the sidebar. --- docs/codebase.rst | 4 ++++ docs/contributinghowto.rst | 4 ++++ docs/designdecisions.rst | 4 ++++ docs/git.rst | 4 ++++ docs/hackinghowto.rst | 4 ++++ 5 files changed, 20 insertions(+) diff --git a/docs/codebase.rst b/docs/codebase.rst index 4f5f215f..898eadfe 100644 --- a/docs/codebase.rst +++ b/docs/codebase.rst @@ -4,6 +4,10 @@ Codebase Documentation ======================== +.. contents:: Sections + :local: + + This chapter covers the libraries that GNU MediaGoblin uses as well as various recipes for getting things done. diff --git a/docs/contributinghowto.rst b/docs/contributinghowto.rst index e980a5e0..06d2814e 100644 --- a/docs/contributinghowto.rst +++ b/docs/contributinghowto.rst @@ -4,6 +4,10 @@ Contributing HOWTO ==================== +.. contents:: Sections + :local: + + .. _join-the-community-section: Join the community! diff --git a/docs/designdecisions.rst b/docs/designdecisions.rst index 50dfe3e8..afa1e26b 100644 --- a/docs/designdecisions.rst +++ b/docs/designdecisions.rst @@ -4,6 +4,10 @@ Design Decisions ================== +.. contents:: Sections + :local: + + This chapter talks a bit about design decisions. diff --git a/docs/git.rst b/docs/git.rst index 6421e093..8eb038b2 100644 --- a/docs/git.rst +++ b/docs/git.rst @@ -2,6 +2,10 @@ Git, Cloning and Patches ========================== +.. contents:: Sections + :local: + + GNU MediaGoblin uses git for all our version control and we have the repositories hosted on `Gitorious `_. We have two repositories: diff --git a/docs/hackinghowto.rst b/docs/hackinghowto.rst index a9aadb62..d8bb9330 100644 --- a/docs/hackinghowto.rst +++ b/docs/hackinghowto.rst @@ -4,6 +4,10 @@ Hacking HOWTO =============== +.. contents:: Sections + :local: + + So you want to hack on GNU MediaGoblin? ======================================= From a68ee5556e2cf78abd1e87546f8627ec07c1f89d Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 13 Jun 2011 21:01:19 -0500 Subject: [PATCH 13/15] A super strict HTML cleaner method with mediocre tests. --- mediagoblin/tests/test_util.py | 19 +++++++++++++++++++ mediagoblin/util.py | 27 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/mediagoblin/tests/test_util.py b/mediagoblin/tests/test_util.py index 7b00a074..75e28aca 100644 --- a/mediagoblin/tests/test_util.py +++ b/mediagoblin/tests/test_util.py @@ -103,3 +103,22 @@ def test_locale_to_lower_lower(): # crazy renditions. Useful? assert util.locale_to_lower_lower('en-US') == 'en-us' assert util.locale_to_lower_lower('en_us') == 'en-us' + + +def test_html_cleaner(): + # Remove images + result = util.clean_html( + '

Hi everybody! ' + '

\n' + '

:)

') + assert result == ( + '
' + '

Hi everybody!

\n' + '

:)

' + '
') + + # Remove evil javascript + result = util.clean_html( + '

innocent link!

') + assert result == ( + '

innocent link!

') diff --git a/mediagoblin/util.py b/mediagoblin/util.py index f29f8570..fc380f41 100644 --- a/mediagoblin/util.py +++ b/mediagoblin/util.py @@ -30,6 +30,7 @@ import jinja2 import translitcodec from paste.deploy.loadwsgi import NicerConfigParser from webob import Response, exc +from lxml.html.clean import Cleaner from mediagoblin import mg_globals from mediagoblin.db.util import ObjectId @@ -373,6 +374,32 @@ def read_config_file(conf_file): return mgoblin_conf +# A super strict version of the lxml.html cleaner class +HTML_CLEANER = Cleaner( + scripts=True, + javascript=True, + comments=True, + style=True, + links=True, + page_structure=True, + processing_instructions=True, + embedded=True, + frames=True, + forms=True, + annoying_tags=True, + allow_tags=[ + 'div', 'b', 'i', 'em', 'strong', 'p', 'ul', 'ol', 'li', 'a', 'br'], + remove_unknown_tags=False, # can't be used with allow_tags + safe_attrs_only=True, + add_nofollow=True, # for now + host_whitelist=(), + whitelist_tags=set([])) + + +def clean_html(html): + return HTML_CLEANER.clean_html(html) + + SETUP_GETTEXTS = {} def setup_gettext(locale): From 8bfa533f8b438e57a7950a8dcd02a275cbfa19df Mon Sep 17 00:00:00 2001 From: Elrond Date: Tue, 14 Jun 2011 20:01:39 +0200 Subject: [PATCH 14/15] Drop WorkbenchManager.localized_file() As Workbench has the localized_file() method, use this everywhere and drop the wrapper method from WorkbenchManager. The processing code already did that. --- mediagoblin/tests/test_workbench.py | 11 +++++------ mediagoblin/workbench.py | 3 --- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/mediagoblin/tests/test_workbench.py b/mediagoblin/tests/test_workbench.py index 2795cd63..db27dfc6 100644 --- a/mediagoblin/tests/test_workbench.py +++ b/mediagoblin/tests/test_workbench.py @@ -83,20 +83,19 @@ class TestWorkbench(object): with this_storage.get_file(filepath, 'w') as our_file: our_file.write('Our file') - filename = self.workbench_manager.localized_file( - this_workbench, this_storage, filepath) + filename = this_workbench.localized_file(this_storage, filepath) assert filename == os.path.join( this_workbench.dir, 'ourfile.txt') # fake remote file storage, filename_if_copying set - filename = self.workbench_manager.localized_file( - this_workbench, this_storage, filepath, 'thisfile') + filename = this_workbench.localized_file( + this_storage, filepath, 'thisfile') assert filename == os.path.join( this_workbench.dir, 'thisfile.txt') # fake remote file storage, filename_if_copying set, # keep_extension_if_copying set to false - filename = self.workbench_manager.localized_file( - this_workbench, this_storage, filepath, 'thisfile.text', False) + filename = this_workbench.localized_file( + this_storage, filepath, 'thisfile.text', False) assert filename == os.path.join( this_workbench.dir, 'thisfile.text') diff --git a/mediagoblin/workbench.py b/mediagoblin/workbench.py index c88b686c..32229d2e 100644 --- a/mediagoblin/workbench.py +++ b/mediagoblin/workbench.py @@ -151,6 +151,3 @@ class WorkbenchManager(object): "Can't destroy workbench outside the base workbench dir") shutil.rmtree(workbench) - - def localized_file(self, workbench, *args, **kwargs): - return workbench.localized_file(*args, **kwargs) From b67a983a02363bd17a4e6a96e650e65aa2d4eb7a Mon Sep 17 00:00:00 2001 From: Elrond Date: Tue, 14 Jun 2011 20:39:14 +0200 Subject: [PATCH 15/15] Move destroy_workbench to Workbench class And add a lot of warnings, as the checks for "being part of the main Manager" are all gone. --- mediagoblin/process_media/__init__.py | 2 +- mediagoblin/tests/test_workbench.py | 15 +++------ mediagoblin/workbench.py | 45 ++++++++++++--------------- 3 files changed, 25 insertions(+), 37 deletions(-) diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index bd067e39..f37bf080 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -75,4 +75,4 @@ def process_media_initial(media_id): entry.save() # clean up workbench - mgg.workbench_manager.destroy_workbench(workbench) + workbench.destroy_self() diff --git a/mediagoblin/tests/test_workbench.py b/mediagoblin/tests/test_workbench.py index db27dfc6..953835a1 100644 --- a/mediagoblin/tests/test_workbench.py +++ b/mediagoblin/tests/test_workbench.py @@ -37,7 +37,7 @@ class TestWorkbench(object): this_workbench = self.workbench_manager.create_workbench() tmpname = this_workbench.joinpath('temp.txt') assert tmpname == os.path.join(this_workbench.dir, 'temp.txt') - self.workbench_manager.destroy_workbench(this_workbench) + this_workbench.destroy_self() def test_destroy_workbench(self): # kill a workbench @@ -49,17 +49,10 @@ class TestWorkbench(object): assert os.path.exists(tmpfile_name) - self.workbench_manager.destroy_workbench(this_workbench) + wb_dir = this_workbench.dir + this_workbench.destroy_self() assert not os.path.exists(tmpfile_name) - assert not os.path.exists(this_workbench.dir) - - # make sure we can't kill other stuff though - dont_kill_this = workbench.Workbench(tempfile.mkdtemp()) - - assert_raises( - workbench.WorkbenchOutsideScope, - self.workbench_manager.destroy_workbench, - dont_kill_this) + assert not os.path.exists(wb_dir) def test_localized_file(self): tmpdir, this_storage = get_tmp_filestorage() diff --git a/mediagoblin/workbench.py b/mediagoblin/workbench.py index 32229d2e..f83c4fa0 100644 --- a/mediagoblin/workbench.py +++ b/mediagoblin/workbench.py @@ -23,24 +23,21 @@ DEFAULT_WORKBENCH_DIR = os.path.join( tempfile.gettempdir(), u'mgoblin_workbench') -# Exception(s) -# ------------ - -class WorkbenchOutsideScope(Exception): - """ - Raised when a workbench is outside a WorkbenchManager scope. - """ - pass - - # Actual workbench stuff # ---------------------- class Workbench(object): """ Represent the directory for the workbench + + WARNING: DO NOT create Workbench objects on your own, + let the WorkbenchManager do that for you! """ def __init__(self, dir): + """ + WARNING: DO NOT create Workbench objects on your own, + let the WorkbenchManager do that for you! + """ self.dir = dir def __unicode__(self): @@ -117,6 +114,19 @@ class Workbench(object): return full_dest_filename + def destroy_self(self): + """ + Destroy this workbench! Deletes the directory and all its contents! + + WARNING: Does no checks for a sane value in self.dir! + """ + # just in case + workbench = os.path.abspath(self.dir) + + shutil.rmtree(workbench) + + del self.dir + class WorkbenchManager(object): """ @@ -136,18 +146,3 @@ class WorkbenchManager(object): Create and return the path to a new workbench (directory). """ return Workbench(tempfile.mkdtemp(dir=self.base_workbench_dir)) - - def destroy_workbench(self, workbench): - """ - Destroy this workbench! Deletes the directory and all its contents! - - Makes sure the workbench actually belongs to this manager though. - """ - # just in case - workbench = os.path.abspath(workbench.dir) - - if not workbench.startswith(self.base_workbench_dir): - raise WorkbenchOutsideScope( - "Can't destroy workbench outside the base workbench dir") - - shutil.rmtree(workbench)