You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2013/09/04 16:31:45 UTC
[1/3] git commit: [#6530] Improved error handling on import_tool task
Updated Branches:
refs/heads/master 19070bda3 -> 153078bbc
[#6530] Improved error handling on import_tool task
Signed-off-by: Cory Johns <cj...@slashdotmedia.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/43f56c26
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/43f56c26
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/43f56c26
Branch: refs/heads/master
Commit: 43f56c26f225c0a5835a0c80c1ed374f240be829
Parents: 19070bd
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Thu Aug 22 20:59:59 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Sep 4 14:30:55 2013 +0000
----------------------------------------------------------------------
Allura/allura/lib/helpers.py | 6 ++++
Allura/allura/tests/test_helpers.py | 34 ++++++++++++++++++++
ForgeImporters/forgeimporters/base.py | 20 +++++++++---
ForgeImporters/forgeimporters/google/tracker.py | 3 ++
.../forgeimporters/tests/google/test_tracker.py | 14 ++++++++
.../forgeimporters/tests/test_base.py | 20 +++++++++++-
.../forgeimporters/trac/tests/test_tickets.py | 33 +++++++++++++++----
ForgeImporters/forgeimporters/trac/tickets.py | 31 ++++++++++--------
8 files changed, 137 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/Allura/allura/lib/helpers.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index adfb339..0e13fad 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -214,6 +214,12 @@ def _make_xs(X, ids):
result = (results.get(i) for i in ids)
return (r for r in result if r is not None)
+def make_app_admin_only(app):
+ from allura.model.auth import ProjectRole
+ admin_role = ProjectRole.by_name('Admin', app.project)
+ for ace in [ace for ace in app.acl if ace.role_id != admin_role._id]:
+ app.acl.remove(ace)
+
@contextmanager
def push_config(obj, **kw):
saved_attrs = {}
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/Allura/allura/tests/test_helpers.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_helpers.py b/Allura/allura/tests/test_helpers.py
index 3d4ad96..e9bc8f9 100644
--- a/Allura/allura/tests/test_helpers.py
+++ b/Allura/allura/tests/test_helpers.py
@@ -26,10 +26,13 @@ from nose.tools import eq_, assert_equals
from IPython.testing.decorators import skipif, module_not_available
from datadiff import tools as dd
from webob import Request
+from ming.orm import ThreadLocalORMSession
from allura import model as M
from allura.lib import helpers as h
from allura.lib.search import inject_user
+from allura.lib.security import has_access
+from allura.lib.security import Credentials
from allura.tests import decorators as td
from alluratest.controller import setup_basic_test
@@ -104,6 +107,37 @@ def test_make_roles():
assert h.make_roles([pr._id]).next() == pr
@td.with_wiki
+def test_make_app_admin_only():
+ h.set_context('test', 'wiki', neighborhood='Projects')
+ anon = M.User.anonymous()
+ dev = M.User.query.get(username='test-user')
+ admin = M.User.query.get(username='test-admin')
+ c.project.add_user(dev, ['Developer'])
+ ThreadLocalORMSession.flush_all()
+ Credentials.get().clear()
+ assert has_access(c.app, 'read', user=anon)()
+ assert has_access(c.app, 'read', user=dev)()
+ assert has_access(c.app, 'read', user=admin)()
+ assert not has_access(c.app, 'create', user=anon)()
+ assert has_access(c.app, 'create', user=dev)()
+ assert has_access(c.app, 'create', user=admin)()
+ assert c.app.is_visible_to(anon)
+ assert c.app.is_visible_to(dev)
+ assert c.app.is_visible_to(admin)
+ h.make_app_admin_only(c.app)
+ ThreadLocalORMSession.flush_all()
+ Credentials.get().clear()
+ assert not has_access(c.app, 'read', user=anon)()
+ assert not has_access(c.app, 'read', user=dev)()
+ assert has_access(c.app, 'read', user=admin)()
+ assert not has_access(c.app, 'create', user=anon)()
+ assert not has_access(c.app, 'create', user=dev)()
+ assert has_access(c.app, 'create', user=admin)()
+ assert not c.app.is_visible_to(anon)
+ assert not c.app.is_visible_to(dev)
+ assert c.app.is_visible_to(admin)
+
+@td.with_wiki
def test_context_setters():
h.set_context('test', 'wiki', neighborhood='Projects')
assert c.project is not None
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/base.py b/ForgeImporters/forgeimporters/base.py
index 7f25e69..69db8d6 100644
--- a/ForgeImporters/forgeimporters/base.py
+++ b/ForgeImporters/forgeimporters/base.py
@@ -19,11 +19,13 @@ import logging
import urllib
import urllib2
from collections import defaultdict
+import traceback
from BeautifulSoup import BeautifulSoup
from tg import expose, validate, flash, redirect, config
from tg.decorators import with_trailing_slash
-from pylons import tmpl_context as c, app_globals as g
+from pylons import app_globals as g
+from pylons import tmpl_context as c
from formencode import validators as fev, schema
from webob import exc
@@ -66,9 +68,19 @@ class ToolImportForm(schema.Schema):
@task(notifications_disabled=True)
def import_tool(importer_name, project_name=None, mount_point=None, mount_label=None, **kw):
- importer = ToolImporter.by_name(importer_name)
- importer.import_tool(c.project, c.user, project_name=project_name,
- mount_point=mount_point, mount_label=mount_label, **kw)
+ try:
+ importer = ToolImporter.by_name(importer_name)
+ importer.import_tool(c.project, c.user, project_name=project_name,
+ mount_point=mount_point, mount_label=mount_label, **kw)
+ except Exception as e:
+ g.post_event('import_tool_task_failed',
+ error=str(e),
+ traceback=traceback.format_exc(),
+ importer_name=importer_name,
+ project_name=project_name,
+ mount_point=mount_point,
+ mount_label=mount_label,
+ **kw)
class ProjectExtractor(object):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/google/tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/google/tracker.py b/ForgeImporters/forgeimporters/google/tracker.py
index b7fccbe..eb97c07 100644
--- a/ForgeImporters/forgeimporters/google/tracker.py
+++ b/ForgeImporters/forgeimporters/google/tracker.py
@@ -135,6 +135,9 @@ class GoogleCodeTrackerImporter(ToolImporter):
g.post_event('project_updated')
app.globals.invalidate_bin_counts()
return app
+ except Exception as e:
+ h.make_app_admin_only(app)
+ raise
finally:
M.session.artifact_orm_session._get().skip_mod_date = False
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/tests/google/test_tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/google/test_tracker.py b/ForgeImporters/forgeimporters/tests/google/test_tracker.py
index 3c7f0aa..5bbf836 100644
--- a/ForgeImporters/forgeimporters/tests/google/test_tracker.py
+++ b/ForgeImporters/forgeimporters/tests/google/test_tracker.py
@@ -82,6 +82,20 @@ class TestTrackerImporter(TestCase):
g.post_event.assert_called_once_with('project_updated')
app.globals.invalidate_bin_counts.assert_called_once_with()
+ @mock.patch.object(tracker, 'ThreadLocalORMSession')
+ @mock.patch.object(tracker, 'M')
+ @mock.patch.object(tracker, 'h')
+ def test_import_tool_failure(self, h, M, ThreadLocalORMSession):
+ h.push_config.side_effect = ValueError
+ project = mock.Mock()
+ user = mock.Mock()
+
+ importer = tracker.GoogleCodeTrackerImporter()
+ self.assertRaises(ValueError, importer.import_tool, project, user, project_name='project_name',
+ mount_point='mount_point', mount_label='mount_label')
+
+ h.make_app_admin_only.assert_called_once_with(project.install_app.return_value)
+
def test_custom_fields(self):
importer = tracker.GoogleCodeTrackerImporter()
importer.custom_fields = {}
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/tests/test_base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/test_base.py b/ForgeImporters/forgeimporters/tests/test_base.py
index 92fb0cc..a61ecda 100644
--- a/ForgeImporters/forgeimporters/tests/test_base.py
+++ b/ForgeImporters/forgeimporters/tests/test_base.py
@@ -42,7 +42,8 @@ class TestProjectExtractor(TestCase):
@mock.patch.object(base.ToolImporter, 'by_name')
@mock.patch.object(base, 'c')
-def test_import_tool(c, by_name):
+@mock.patch.object(base, 'g')
+def test_import_tool(g, c, by_name):
c.project = mock.Mock(name='project')
c.user = mock.Mock(name='user')
base.import_tool('importer_name', project_name='project_name',
@@ -51,6 +52,23 @@ def test_import_tool(c, by_name):
by_name.return_value.import_tool.assert_called_once_with(c.project,
c.user, project_name='project_name', mount_point='mount_point',
mount_label='mount_label')
+ assert not g.post_event.called
+
+@mock.patch.object(base.ToolImporter, 'by_name')
+@mock.patch.object(base, 'g')
+def test_import_tool_failed(g, by_name):
+ by_name.side_effect = RuntimeError('my error')
+ base.import_tool('importer_name', project_name='project_name',
+ mount_point='mount_point', mount_label='mount_label', other='other')
+ g.post_event.assert_called_once_with(
+ 'import_tool_task_failed',
+ error=by_name.side_effect,
+ importer_name='importer_name',
+ project_name='project_name',
+ mount_point='mount_point',
+ mount_label='mount_label',
+ other='other',
+ )
def ep(name, source=None, importer=None, **kw):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/trac/tests/test_tickets.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/trac/tests/test_tickets.py b/ForgeImporters/forgeimporters/trac/tests/test_tickets.py
index 953c6d7..74e1049 100644
--- a/ForgeImporters/forgeimporters/trac/tests/test_tickets.py
+++ b/ForgeImporters/forgeimporters/trac/tests/test_tickets.py
@@ -46,12 +46,13 @@ class TestTracTicketImporter(TestCase):
project = Mock(name='Project', shortname='myproject')
project.install_app.return_value = app
user = Mock(name='User', _id='id')
- res = importer.import_tool(project, user,
- mount_point='bugs',
- mount_label='Bugs',
- trac_url='http://example.com/trac/url',
- user_map=json.dumps(user_map),
- )
+ with patch.dict('forgeimporters.trac.tickets.config', {'base_url': 'foo'}):
+ res = importer.import_tool(project, user,
+ mount_point='bugs',
+ mount_label='Bugs',
+ trac_url='http://example.com/trac/url',
+ user_map=json.dumps(user_map),
+ )
self.assertEqual(res, app)
project.install_app.assert_called_once_with(
'Tickets', mount_point='bugs', mount_label='Bugs')
@@ -68,6 +69,26 @@ class TestTracTicketImporter(TestCase):
validate=False)
g.post_event.assert_called_once_with('project_updated')
+ @patch('forgeimporters.trac.tickets.session')
+ @patch('forgeimporters.trac.tickets.h')
+ @patch('forgeimporters.trac.tickets.TracExport')
+ def test_import_tool_failure(self, TracExport, h, session):
+ importer = TracTicketImporter()
+ app = Mock(name='ForgeTrackerApp')
+ project = Mock(name='Project', shortname='myproject')
+ project.install_app.return_value = app
+ user = Mock(name='User', _id='id')
+ TracExport.side_effect = ValueError
+
+ self.assertRaises(ValueError, importer.import_tool, project, user,
+ mount_point='bugs',
+ mount_label='Bugs',
+ trac_url='http://example.com/trac/url',
+ user_map=None,
+ )
+
+ h.make_app_admin_only.assert_called_once_with(app)
+
class TestTracTicketImportController(TestController, TestCase):
def setUp(self):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/43f56c26/ForgeImporters/forgeimporters/trac/tickets.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/trac/tickets.py b/ForgeImporters/forgeimporters/trac/tickets.py
index 3e0e486..941ff32 100644
--- a/ForgeImporters/forgeimporters/trac/tickets.py
+++ b/ForgeImporters/forgeimporters/trac/tickets.py
@@ -42,6 +42,7 @@ from allura.controllers import BaseController
from allura.lib.decorators import require_post, task
from allura.lib.import_api import AlluraImportApiClient
from allura.lib import validators as v
+from allura.lib import helpers as h
from allura.model import ApiTicket
from allura.scripts.trac_export import (
TracExport,
@@ -116,16 +117,20 @@ class TracTicketImporter(ToolImporter):
)
session(app.config).flush(app.config)
session(app.globals).flush(app.globals)
- export = [ticket for ticket in TracExport(trac_url)]
- export_string = json.dumps(export, cls=DateJSONEncoder)
- api_ticket = ApiTicket(user_id=user._id,
- capabilities={"import": ["Projects", project.shortname]},
- expires=datetime.utcnow() + timedelta(minutes=60))
- session(api_ticket).flush(api_ticket)
- cli = AlluraImportApiClient(config['base_url'], api_ticket.api_key,
- api_ticket.secret_key, verbose=True)
- import_tracker(cli, project.shortname, mount_point,
- {'user_map': json.loads(user_map) if user_map else {}},
- export_string, validate=False)
- g.post_event('project_updated')
- return app
+ try:
+ export = [ticket for ticket in TracExport(trac_url)]
+ export_string = json.dumps(export, cls=DateJSONEncoder)
+ api_ticket = ApiTicket(user_id=user._id,
+ capabilities={"import": ["Projects", project.shortname]},
+ expires=datetime.utcnow() + timedelta(minutes=60))
+ session(api_ticket).flush(api_ticket)
+ cli = AlluraImportApiClient(config['base_url'], api_ticket.api_key,
+ api_ticket.secret_key, verbose=True)
+ import_tracker(cli, project.shortname, mount_point,
+ {'user_map': json.loads(user_map) if user_map else {}},
+ export_string, validate=False)
+ g.post_event('project_updated')
+ return app
+ except Exception as e:
+ h.make_app_admin_only(app)
+ raise
[3/3] git commit: [#6530] Refactor error handling into context manager
Posted by br...@apache.org.
[#6530] Refactor error handling into context manager
- Also allows exc to propagate up so task will end in error state
Signed-off-by: Tim Van Steenburgh <tv...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/153078bb
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/153078bb
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/153078bb
Branch: refs/heads/master
Commit: 153078bbc7cf0f76c2bd13b155e7a3c09024d9eb
Parents: 1794af5
Author: Tim Van Steenburgh <tv...@gmail.com>
Authored: Tue Sep 3 20:15:27 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Sep 4 14:30:57 2013 +0000
----------------------------------------------------------------------
ForgeImporters/forgeimporters/base.py | 31 +++++++++++++-------
ForgeImporters/forgeimporters/google/code.py | 18 ++++--------
ForgeImporters/forgeimporters/google/tracker.py | 14 ++-------
.../forgeimporters/tests/test_base.py | 5 ++--
ForgeImporters/forgeimporters/trac/tickets.py | 14 ++-------
5 files changed, 36 insertions(+), 46 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/153078bb/ForgeImporters/forgeimporters/base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/base.py b/ForgeImporters/forgeimporters/base.py
index 54f006b..e0e9d6f 100644
--- a/ForgeImporters/forgeimporters/base.py
+++ b/ForgeImporters/forgeimporters/base.py
@@ -66,20 +66,31 @@ class ToolImportForm(schema.Schema):
mount_label = fev.UnicodeString()
+class ImportErrorHandler(object):
+ def __init__(self, importer, project_name):
+ self.importer = importer
+ self.project_name = project_name
+
+ def __enter__(self):
+ pass
+
+ def __exit__(self, exc_type, exc_val, exc_tb):
+ if exc_type:
+ g.post_event('import_tool_task_failed',
+ error=str(exc_val),
+ traceback=traceback.format_exc(),
+ importer_source=self.importer.source,
+ importer_tool_label=self.importer.tool_label,
+ project_name=self.project_name,
+ )
+
+
@task(notifications_disabled=True)
def import_tool(importer_name, project_name=None, mount_point=None, mount_label=None, **kw):
- try:
- importer = ToolImporter.by_name(importer_name)
+ importer = ToolImporter.by_name(importer_name)
+ with ImportErrorHandler(importer, project_name):
importer.import_tool(c.project, c.user, project_name=project_name,
mount_point=mount_point, mount_label=mount_label, **kw)
- except Exception as e:
- g.post_event('import_tool_task_failed',
- error=str(e),
- traceback=traceback.format_exc(),
- importer_source=importer.source,
- importer_tool_label=importer.tool_label,
- project_name=project_name,
- )
class ProjectExtractor(object):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/153078bb/ForgeImporters/forgeimporters/google/code.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/google/code.py b/ForgeImporters/forgeimporters/google/code.py
index c270ee0..4cf752d 100644
--- a/ForgeImporters/forgeimporters/google/code.py
+++ b/ForgeImporters/forgeimporters/google/code.py
@@ -16,7 +16,6 @@
# under the License.
import urllib2
-import traceback
import formencode as fe
from formencode import validators as fev
@@ -38,7 +37,10 @@ from allura.controllers import BaseController
from allura.lib import validators as v
from allura.lib.decorators import require_post, task
-from forgeimporters.base import ToolImporter
+from forgeimporters.base import (
+ ToolImporter,
+ ImportErrorHandler,
+ )
from forgeimporters.google import GoogleCodeProjectExtractor
REPO_APPS = {}
@@ -84,17 +86,9 @@ def get_repo_class(type_):
@task(notifications_disabled=True)
def import_tool(**kw):
- try:
- importer = GoogleRepoImporter()
+ importer = GoogleRepoImporter()
+ with ImportErrorHandler(importer, kw.get('project_name')):
importer.import_tool(c.project, c.user, **kw)
- except Exception as e:
- g.post_event('import_tool_task_failed',
- error=str(e),
- traceback=traceback.format_exc(),
- importer_source=importer.source,
- importer_tool_label=importer.tool_label,
- project_name=kw.get('project_name'),
- )
class GoogleRepoImportForm(fe.schema.Schema):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/153078bb/ForgeImporters/forgeimporters/google/tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/google/tracker.py b/ForgeImporters/forgeimporters/google/tracker.py
index e5c8dc3..1535531 100644
--- a/ForgeImporters/forgeimporters/google/tracker.py
+++ b/ForgeImporters/forgeimporters/google/tracker.py
@@ -17,7 +17,6 @@
from collections import defaultdict
from datetime import datetime
-import traceback
from formencode import validators as fev
@@ -49,22 +48,15 @@ from . import GoogleCodeProjectExtractor
from forgeimporters.base import (
ToolImporter,
ToolImportForm,
+ ImportErrorHandler,
)
@task(notifications_disabled=True)
def import_tool(**kw):
- try:
- importer = GoogleCodeTrackerImporter()
+ importer = GoogleCodeTrackerImporter()
+ with ImportErrorHandler(importer, kw.get('project_name')):
importer.import_tool(c.project, c.user, **kw)
- except Exception as e:
- g.post_event('import_tool_task_failed',
- error=str(e),
- traceback=traceback.format_exc(),
- importer_source=importer.source,
- importer_tool_label=importer.tool_label,
- project_name=kw.get('project_name'),
- )
class GoogleCodeTrackerImportForm(ToolImportForm):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/153078bb/ForgeImporters/forgeimporters/tests/test_base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/test_base.py b/ForgeImporters/forgeimporters/tests/test_base.py
index f6d443f..7de0625 100644
--- a/ForgeImporters/forgeimporters/tests/test_base.py
+++ b/ForgeImporters/forgeimporters/tests/test_base.py
@@ -20,7 +20,7 @@ from unittest import TestCase
from formencode import Invalid
import mock
from tg import expose
-from nose.tools import assert_equal
+from nose.tools import assert_equal, assert_raises
from alluratest.controller import TestController
@@ -66,7 +66,8 @@ def test_import_tool_failed(g, ToolImporter, format_exc):
importer.import_tool.side_effect = RuntimeError('my error')
ToolImporter.by_name.return_value = importer
- base.import_tool('importer_name', project_name='project_name')
+ assert_raises(RuntimeError, base.import_tool, 'importer_name',
+ project_name='project_name')
g.post_event.assert_called_once_with(
'import_tool_task_failed',
error=str(importer.import_tool.side_effect),
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/153078bb/ForgeImporters/forgeimporters/trac/tickets.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/trac/tickets.py b/ForgeImporters/forgeimporters/trac/tickets.py
index 7876b44..3063d71 100644
--- a/ForgeImporters/forgeimporters/trac/tickets.py
+++ b/ForgeImporters/forgeimporters/trac/tickets.py
@@ -20,7 +20,6 @@ from datetime import (
timedelta,
)
import json
-import traceback
from formencode import validators as fev
@@ -53,6 +52,7 @@ from allura.scripts.trac_export import (
from forgeimporters.base import (
ToolImporter,
ToolImportForm,
+ ImportErrorHandler,
)
from forgetracker.tracker_main import ForgeTrackerApp
from forgetracker.scripts.import_tracker import import_tracker
@@ -60,17 +60,9 @@ from forgetracker.scripts.import_tracker import import_tracker
@task(notifications_disabled=True)
def import_tool(**kw):
- try:
- importer = TracTicketImporter()
+ importer = TracTicketImporter()
+ with ImportErrorHandler(importer, kw.get('trac_url')):
importer.import_tool(c.project, c.user, **kw)
- except Exception as e:
- g.post_event('import_tool_task_failed',
- error=str(e),
- traceback=traceback.format_exc(),
- importer_source=importer.source,
- importer_tool_label=importer.tool_label,
- project_name=kw.get('trac_url'),
- )
class TracTicketImportForm(ToolImportForm):
[2/3] git commit: [#6530] Add error handling to individual tool
importers
Posted by br...@apache.org.
[#6530] Add error handling to individual tool importers
Signed-off-by: Tim Van Steenburgh <tv...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/1794af54
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/1794af54
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/1794af54
Branch: refs/heads/master
Commit: 1794af54ad7e92714cb27679fb50df38602fe6b4
Parents: 43f56c2
Author: Tim Van Steenburgh <tv...@gmail.com>
Authored: Fri Aug 30 16:49:24 2013 +0000
Committer: Dave Brondsema <db...@slashdotmedia.com>
Committed: Wed Sep 4 14:30:57 2013 +0000
----------------------------------------------------------------------
ForgeImporters/forgeimporters/base.py | 7 +++---
ForgeImporters/forgeimporters/google/code.py | 13 +++++++++-
ForgeImporters/forgeimporters/google/tracker.py | 13 +++++++++-
.../forgeimporters/tests/test_base.py | 26 ++++++++++++--------
ForgeImporters/forgeimporters/trac/tickets.py | 13 +++++++++-
5 files changed, 55 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/1794af54/ForgeImporters/forgeimporters/base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/base.py b/ForgeImporters/forgeimporters/base.py
index 69db8d6..54f006b 100644
--- a/ForgeImporters/forgeimporters/base.py
+++ b/ForgeImporters/forgeimporters/base.py
@@ -76,11 +76,10 @@ def import_tool(importer_name, project_name=None, mount_point=None, mount_label=
g.post_event('import_tool_task_failed',
error=str(e),
traceback=traceback.format_exc(),
- importer_name=importer_name,
+ importer_source=importer.source,
+ importer_tool_label=importer.tool_label,
project_name=project_name,
- mount_point=mount_point,
- mount_label=mount_label,
- **kw)
+ )
class ProjectExtractor(object):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/1794af54/ForgeImporters/forgeimporters/google/code.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/google/code.py b/ForgeImporters/forgeimporters/google/code.py
index ed2519f..c270ee0 100644
--- a/ForgeImporters/forgeimporters/google/code.py
+++ b/ForgeImporters/forgeimporters/google/code.py
@@ -16,6 +16,7 @@
# under the License.
import urllib2
+import traceback
import formencode as fe
from formencode import validators as fev
@@ -83,7 +84,17 @@ def get_repo_class(type_):
@task(notifications_disabled=True)
def import_tool(**kw):
- GoogleRepoImporter().import_tool(c.project, c.user, **kw)
+ try:
+ importer = GoogleRepoImporter()
+ importer.import_tool(c.project, c.user, **kw)
+ except Exception as e:
+ g.post_event('import_tool_task_failed',
+ error=str(e),
+ traceback=traceback.format_exc(),
+ importer_source=importer.source,
+ importer_tool_label=importer.tool_label,
+ project_name=kw.get('project_name'),
+ )
class GoogleRepoImportForm(fe.schema.Schema):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/1794af54/ForgeImporters/forgeimporters/google/tracker.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/google/tracker.py b/ForgeImporters/forgeimporters/google/tracker.py
index eb97c07..e5c8dc3 100644
--- a/ForgeImporters/forgeimporters/google/tracker.py
+++ b/ForgeImporters/forgeimporters/google/tracker.py
@@ -17,6 +17,7 @@
from collections import defaultdict
from datetime import datetime
+import traceback
from formencode import validators as fev
@@ -53,7 +54,17 @@ from forgeimporters.base import (
@task(notifications_disabled=True)
def import_tool(**kw):
- GoogleCodeTrackerImporter().import_tool(c.project, c.user, **kw)
+ try:
+ importer = GoogleCodeTrackerImporter()
+ importer.import_tool(c.project, c.user, **kw)
+ except Exception as e:
+ g.post_event('import_tool_task_failed',
+ error=str(e),
+ traceback=traceback.format_exc(),
+ importer_source=importer.source,
+ importer_tool_label=importer.tool_label,
+ project_name=kw.get('project_name'),
+ )
class GoogleCodeTrackerImportForm(ToolImportForm):
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/1794af54/ForgeImporters/forgeimporters/tests/test_base.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/tests/test_base.py b/ForgeImporters/forgeimporters/tests/test_base.py
index a61ecda..f6d443f 100644
--- a/ForgeImporters/forgeimporters/tests/test_base.py
+++ b/ForgeImporters/forgeimporters/tests/test_base.py
@@ -54,20 +54,26 @@ def test_import_tool(g, c, by_name):
mount_label='mount_label')
assert not g.post_event.called
-@mock.patch.object(base.ToolImporter, 'by_name')
+
+@mock.patch.object(base.traceback, 'format_exc')
+@mock.patch.object(base, 'ToolImporter')
@mock.patch.object(base, 'g')
-def test_import_tool_failed(g, by_name):
- by_name.side_effect = RuntimeError('my error')
- base.import_tool('importer_name', project_name='project_name',
- mount_point='mount_point', mount_label='mount_label', other='other')
+def test_import_tool_failed(g, ToolImporter, format_exc):
+ format_exc.return_value = 'my traceback'
+
+ importer = mock.Mock(source='importer_source',
+ tool_label='importer_tool_label')
+ importer.import_tool.side_effect = RuntimeError('my error')
+ ToolImporter.by_name.return_value = importer
+
+ base.import_tool('importer_name', project_name='project_name')
g.post_event.assert_called_once_with(
'import_tool_task_failed',
- error=by_name.side_effect,
- importer_name='importer_name',
+ error=str(importer.import_tool.side_effect),
+ traceback='my traceback',
+ importer_source='importer_source',
+ importer_tool_label='importer_tool_label',
project_name='project_name',
- mount_point='mount_point',
- mount_label='mount_label',
- other='other',
)
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/1794af54/ForgeImporters/forgeimporters/trac/tickets.py
----------------------------------------------------------------------
diff --git a/ForgeImporters/forgeimporters/trac/tickets.py b/ForgeImporters/forgeimporters/trac/tickets.py
index 941ff32..7876b44 100644
--- a/ForgeImporters/forgeimporters/trac/tickets.py
+++ b/ForgeImporters/forgeimporters/trac/tickets.py
@@ -20,6 +20,7 @@ from datetime import (
timedelta,
)
import json
+import traceback
from formencode import validators as fev
@@ -59,7 +60,17 @@ from forgetracker.scripts.import_tracker import import_tracker
@task(notifications_disabled=True)
def import_tool(**kw):
- TracTicketImporter().import_tool(c.project, c.user, **kw)
+ try:
+ importer = TracTicketImporter()
+ importer.import_tool(c.project, c.user, **kw)
+ except Exception as e:
+ g.post_event('import_tool_task_failed',
+ error=str(e),
+ traceback=traceback.format_exc(),
+ importer_source=importer.source,
+ importer_tool_label=importer.tool_label,
+ project_name=kw.get('trac_url'),
+ )
class TracTicketImportForm(ToolImportForm):