Return-Path: X-Original-To: apmail-incubator-allura-commits-archive@minotaur.apache.org Delivered-To: apmail-incubator-allura-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 0C497105B4 for ; Thu, 9 Jan 2014 21:32:12 +0000 (UTC) Received: (qmail 87372 invoked by uid 500); 9 Jan 2014 21:32:11 -0000 Delivered-To: apmail-incubator-allura-commits-archive@incubator.apache.org Received: (qmail 87352 invoked by uid 500); 9 Jan 2014 21:32:11 -0000 Mailing-List: contact allura-commits-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: allura-dev@incubator.apache.org Delivered-To: mailing list allura-commits@incubator.apache.org Received: (qmail 87344 invoked by uid 99); 9 Jan 2014 21:32:11 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Jan 2014 21:32:11 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id A56EF8B449A; Thu, 9 Jan 2014 21:32:11 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: johnsca@apache.org To: allura-commits@incubator.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: git commit: [#6993] Fixed hidden arg error prevent GH attachments from importing Date: Thu, 9 Jan 2014 21:32:11 +0000 (UTC) Updated Branches: refs/heads/cj/6993 [created] 7f9f8da1c [#6993] Fixed hidden arg error prevent GH attachments from importing Signed-off-by: Cory Johns Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/7f9f8da1 Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/7f9f8da1 Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/7f9f8da1 Branch: refs/heads/cj/6993 Commit: 7f9f8da1c39bcd5169c0f9833e842eac71fe1a83 Parents: da8258d Author: Cory Johns Authored: Thu Jan 9 21:31:48 2014 +0000 Committer: Cory Johns Committed: Thu Jan 9 21:31:48 2014 +0000 ---------------------------------------------------------------------- ForgeImporters/forgeimporters/github/tracker.py | 38 ++++++++++++++------ .../forgeimporters/tests/github/test_tracker.py | 25 +++++++++++-- 2 files changed, 49 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7f9f8da1/ForgeImporters/forgeimporters/github/tracker.py ---------------------------------------------------------------------- diff --git a/ForgeImporters/forgeimporters/github/tracker.py b/ForgeImporters/forgeimporters/github/tracker.py index 31ed344..db16c83 100644 --- a/ForgeImporters/forgeimporters/github/tracker.py +++ b/ForgeImporters/forgeimporters/github/tracker.py @@ -16,7 +16,9 @@ # under the License. import re +import logging from datetime import datetime +from urllib2 import HTTPError try: from cStringIO import StringIO @@ -52,6 +54,9 @@ from forgeimporters.base import ToolImportForm from forgeimporters.github.utils import GitHubMarkdownConverter +log = logging.getLogger(__name__) + + class GitHubTrackerImportForm(ToolImportForm): gh_project_name = fev.UnicodeString(not_empty=True) gh_user_name = fev.UnicodeString(not_empty=True) @@ -127,7 +132,7 @@ class GitHubTrackerImporter(ToolImporter): ticket_num=ticket_num, import_id=import_id_converter.expand(ticket_num, app) ) - self.process_fields(ticket, issue) + self.process_fields(extractor, ticket, issue) self.process_comments(extractor, ticket, issue) self.process_events(extractor, ticket, issue) self.process_milestones(ticket, issue) @@ -153,7 +158,7 @@ class GitHubTrackerImporter(ToolImporter): def get_user_link(self, user): return u'[{0}](https://github.com/{0})'.format(user) - def process_fields(self, ticket, issue): + def process_fields(self, extractor, ticket, issue): ticket.summary = issue['title'] ticket.status = issue['state'] ticket.created_date = self.parse_datetime(issue['created_at']) @@ -164,7 +169,7 @@ class GitHubTrackerImporter(ToolImporter): else: owner_line = '' # body processing happens here - body, attachments = self._get_attachments(issue['body']) + body, attachments = self._get_attachments(extractor, issue['body']) ticket.add_multiple_attachments(attachments) ticket.description = ( u'*Originally created by:* {creator}\n' @@ -179,7 +184,7 @@ class GitHubTrackerImporter(ToolImporter): def process_comments(self, extractor, ticket, issue): for comment in extractor.iter_comments(issue): - body, attachments = self._get_attachments(comment['body']) + body, attachments = self._get_attachments(extractor, comment['body']) if comment['user']: posted_by = u'*Originally posted by:* {}\n\n'.format( self.get_user_link(comment['user']['login'])) @@ -240,7 +245,7 @@ class GitHubTrackerImporter(ToolImporter): }) return [global_milestones] - def _get_attachments(self, body): + def _get_attachments(self, extractor, body): # at github, attachments are images only and are included into comment's body # usual syntax is # ![cdbpzjc5ex4](https://f.cloud.github.com/assets/979771/1027411/a393ab5e-0e70-11e3-8a38-b93a3df904cf.jpg)\r\n @@ -258,19 +263,30 @@ class GitHubTrackerImporter(ToolImporter): body = body.replace(match.group(0), '') # stripping url and extension attachments.append(Attachment( + extractor, match.group(1), # url 'attach{}.{}'.format(i + 1, match.group(2)) # extension )) return (body, attachments) class Attachment(object): - def __init__(self, url, filename): + def __init__(self, extractor, url, filename): self.url = url self.filename = filename self.type = None + file = self.get_file(extractor) + if file: + # don't set unless valid (add_multiple_attachments uses hasattr) + self.file = file - @property - def file(self): - fp_ish = GitHubProjectExtractor.urlopen(self.url) - fp = StringIO(fp_ish.read()) - return fp + def get_file(self, extractor): + try: + fp_ish = extractor.urlopen(self.url) + fp = StringIO(fp_ish.read()) + return fp + except HTTPError as e: + if e.code == 404: + log.error('Unable to load attachment: %s', self.url) + return None + else: + raise http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7f9f8da1/ForgeImporters/forgeimporters/tests/github/test_tracker.py ---------------------------------------------------------------------- diff --git a/ForgeImporters/forgeimporters/tests/github/test_tracker.py b/ForgeImporters/forgeimporters/tests/github/test_tracker.py index 9679c6b..b32df0a 100644 --- a/ForgeImporters/forgeimporters/tests/github/test_tracker.py +++ b/ForgeImporters/forgeimporters/tests/github/test_tracker.py @@ -17,6 +17,7 @@ from datetime import datetime from unittest import TestCase +from urllib2 import HTTPError import mock from ...github import tracker @@ -80,9 +81,11 @@ class TestTrackerImporter(TestCase): } importer = tracker.GitHubTrackerImporter() importer.github_markdown_converter = GitHubMarkdownConverter('user', 'project') + extractor = mock.Mock() + extractor.urlopen().read.return_value = 'data' with mock.patch.object(tracker, 'datetime') as dt: dt.strptime.side_effect = lambda s,f: s - importer.process_fields(ticket, issue) + importer.process_fields(extractor, ticket, issue) self.assertEqual(ticket.summary, 'title') self.assertEqual(ticket.description, '*Originally created by:* [creator](https://github.com/creator)\n*Originally owned by:* [owner](https://github.com/owner)\n\nhello') self.assertEqual(ticket.status, 'New') @@ -116,14 +119,28 @@ class TestTrackerImporter(TestCase): def test_get_attachments(self): importer = tracker.GitHubTrackerImporter() + extractor = mock.Mock() + extractor.urlopen().read.return_value = 'data' body = 'hello\n' \ '![cdbpzjc5ex4](https://f.cloud.github.com/assets/979771/1027411/a393ab5e-0e70-11e3-8a38-b93a3df904cf.jpg)\r\n' \ '![screensh0t](http://f.cl.ly/items/13453x43053r2G0d3x0v/Screen%20Shot%202012-04-28%20at%2010.48.17%20AM.png)' - new_body, attachments = importer._get_attachments(body) + new_body, attachments = importer._get_attachments(extractor, body) self.assertEqual(new_body, 'hello\n') self.assertEqual(len(attachments), 2) self.assertEqual(attachments[0].url, 'https://f.cloud.github.com/assets/979771/1027411/a393ab5e-0e70-11e3-8a38-b93a3df904cf.jpg') self.assertEqual(attachments[1].url, 'http://f.cl.ly/items/13453x43053r2G0d3x0v/Screen%20Shot%202012-04-28%20at%2010.48.17%20AM.png') + self.assertEqual(attachments[0].file.read(), 'data') + self.assertEqual(attachments[1].file.read(), 'data') + + def test_get_attachments_404(self): + importer = tracker.GitHubTrackerImporter() + extractor = mock.Mock() + extractor.urlopen.side_effect = HTTPError('url', 404, 'mock', None, None) + body = 'hello\n' \ + '![cdbpzjc5ex4](https://f.cloud.github.com/assets/979771/1027411/a393ab5e-0e70-11e3-8a38-b93a3df904cf.jpg)\r\n' + new_body, attachments = importer._get_attachments(extractor, body) + self.assertIsNotNone(attachments[0]) + assert not hasattr(attachments[0], 'file') def test_process_comments(self): ticket = mock.Mock() @@ -213,9 +230,11 @@ Hello } importer = tracker.GitHubTrackerImporter() importer.github_markdown_converter = GitHubMarkdownConverter('user', 'project') + extractor = mock.Mock() + extractor.urlopen().read.return_value = 'data' with mock.patch.object(tracker, 'datetime') as dt: dt.strptime.side_effect = lambda s,f: s - importer.process_fields(ticket, issue) + importer.process_fields(extractor, ticket, issue) self.assertEqual(ticket.description.strip(), body_converted.strip()) def test_github_markdown_converted_in_comments(self):