From 6046a852053cd9543637d5c3b1d8c2a493daaf55 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Mon, 27 May 2013 15:17:00 +0100 Subject: [PATCH 1/4] Make ``get_feature_info`` of github providers marginally better. --- deploystream/apps/feature/lib.py | 13 +++++-- deploystream/apps/feature/views.py | 5 ++- deploystream/exceptions.py | 4 ++ deploystream/providers/github/__init__.py | 47 ++++++++++++++--------- tests/test_feature/test_view.py | 1 + 5 files changed, 47 insertions(+), 23 deletions(-) diff --git a/deploystream/apps/feature/lib.py b/deploystream/apps/feature/lib.py index a4ad4b0..2bbdff7 100644 --- a/deploystream/apps/feature/lib.py +++ b/deploystream/apps/feature/lib.py @@ -2,7 +2,8 @@ #-*- coding: utf-8 -*- from deploystream import app -from deploystream.exceptions import UnknownProviderException +from deploystream.exceptions import ( + UnknownProviderException, UnknownFeatureException) from deploystream.providers.interfaces import ( IBuildInfoProvider, IPlanningProvider, ISourceCodeControlProvider) from .models import Branch, BuildInfo, Feature @@ -40,14 +41,20 @@ def get_feature_info(feature_provider, feature_id, providers): :raises: UnknownProviderException - if no such name found. """ + print providers if feature_provider not in providers: raise UnknownProviderException(feature_provider) # First get feature info from the management provider planning_provider = providers[feature_provider] - feature = Feature(planning_provider, - **planning_provider.get_feature_info(feature_id)) + feature_info = planning_provider.get_feature_info(feature_id) + print planning_provider, feature_info + if not feature_info: + raise UnknownFeatureException(feature_id) + + feature = Feature(planning_provider, **feature_info) + # Then get any branch info from any source control providers for provider in providers[ISourceCodeControlProvider]: for branch_data in provider.get_repo_branches_involved( diff --git a/deploystream/apps/feature/views.py b/deploystream/apps/feature/views.py index 28c7bd0..0ff2441 100644 --- a/deploystream/apps/feature/views.py +++ b/deploystream/apps/feature/views.py @@ -6,7 +6,8 @@ from deploystream.apps.feature.lib import get_feature_info, get_all_features from deploystream.lib.transforms import nativify from deploystream.decorators import needs_providers -from deploystream.exceptions import UnknownProviderException +from deploystream.exceptions import ( + UnknownProviderException, UnknownFeatureException) def as_json(func): @@ -38,4 +39,6 @@ def view_feature(source_id, feature_id, providers): feature = get_feature_info(source_id, feature_id, providers) except UnknownProviderException: abort(404) + except UnknownFeatureException: + abort(404) return feature diff --git a/deploystream/exceptions.py b/deploystream/exceptions.py index 0f61630..91b8069 100644 --- a/deploystream/exceptions.py +++ b/deploystream/exceptions.py @@ -6,3 +6,7 @@ def __init__(self, missing_token, *args, **kwargs): class UnknownProviderException(Exception): pass + + +class UnknownFeatureException(Exception): + pass diff --git a/deploystream/providers/github/__init__.py b/deploystream/providers/github/__init__.py index d451bb8..411e059 100644 --- a/deploystream/providers/github/__init__.py +++ b/deploystream/providers/github/__init__.py @@ -70,19 +70,7 @@ def get_features(self, **filters): repository.name) if repository.has_issues: for issue in repository.iter_issues(**filters): - issue_info = transforms.remap(issue.__dict__, FEATURE_MAP) - if issue.pull_request: - issue_type = 'PR' - else: - issue_type = 'story' - issue_info['type'] = issue_type - issue_info['project'] = project - owner = issue_info['assignee'] - if owner is None: - issue_info['owner'] = '' - else: - # take only login name from User object - issue_info['owner'] = owner.login + issue_info = self._convert_to_dict(issue, project) features.append(issue_info) # sort by putting PRs first, stories second @@ -90,13 +78,34 @@ def get_features(self, **filters): return features + def _convert_to_dict(self, issue, project): + issue_info = transforms.remap(issue.__dict__, FEATURE_MAP) + if issue.pull_request: + issue_type = 'PR' + else: + issue_type = 'story' + issue_info['type'] = issue_type + issue_info['project'] = project + owner = issue_info['assignee'] + if owner is None: + issue_info['owner'] = '' + else: + # take only login name from User object + issue_info['owner'] = owner.login + return issue_info + def get_feature_info(self, feature_id): - # Feature ID will need to have org in it. - # For now we'll do a really crude search through the get_features - # results - for feat in self.get_features(): - if str(feat['id']) == str(feature_id): - return feat + # Issue with this approach is that we return the first issue with an + # ID across all repos. + # Such are the shortcomings of using Git as a planning provider. To + # get round this we'd need to have the repo in the feature_id, but this + # seems a bad idea from the POV of matching branch names. + for repository in self.repositories: + project = '{0}/{1}'.format(repository.owner.login, + repository.name) + issue = repository.issue(feature_id) + if issue: + return self._convert_to_dict(issue, project) @classmethod def get_oauth_data(self): diff --git a/tests/test_feature/test_view.py b/tests/test_feature/test_view.py index 4e7a902..fc2619b 100644 --- a/tests/test_feature/test_view.py +++ b/tests/test_feature/test_view.py @@ -82,6 +82,7 @@ def setUp(self): 'testbuild': BuildInfoProvider}) def test_feature_view_shows_details(self): response = self.client.get('/features/plan/FT101') + print response.__dict__ feature_dict = json.loads(response.data) assert_true("Amazing feature that will blow your mind" in feature_dict['title']) From 778c8ed7b3448eb305c26c3e13971f35291b08f6 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Mon, 27 May 2013 16:04:19 +0100 Subject: [PATCH 2/4] Complete Jira implementation. Add basic 60 day filter for github. --- deploystream/apps/feature/lib.py | 3 +-- deploystream/providers/github/__init__.py | 19 ++++++++++++---- deploystream/providers/jira/__init__.py | 27 +++++++++++++++++++++-- requirements/runtime.txt | 1 + tests/test_feature/test_view.py | 1 - 5 files changed, 42 insertions(+), 9 deletions(-) diff --git a/deploystream/apps/feature/lib.py b/deploystream/apps/feature/lib.py index 2bbdff7..2c85afa 100644 --- a/deploystream/apps/feature/lib.py +++ b/deploystream/apps/feature/lib.py @@ -41,7 +41,6 @@ def get_feature_info(feature_provider, feature_id, providers): :raises: UnknownProviderException - if no such name found. """ - print providers if feature_provider not in providers: raise UnknownProviderException(feature_provider) @@ -49,7 +48,7 @@ def get_feature_info(feature_provider, feature_id, providers): planning_provider = providers[feature_provider] feature_info = planning_provider.get_feature_info(feature_id) - print planning_provider, feature_info + if not feature_info: raise UnknownFeatureException(feature_id) diff --git a/deploystream/providers/github/__init__.py b/deploystream/providers/github/__init__.py index 411e059..86cfc1f 100644 --- a/deploystream/providers/github/__init__.py +++ b/deploystream/providers/github/__init__.py @@ -1,5 +1,8 @@ -import github3 +import datetime +import iso8601 import re + +import github3 from zope import interface from deploystream.providers.interfaces import IPlanningProvider @@ -144,6 +147,7 @@ def get_repo_branches_involved(self, feature_id, hierarchy_regexes): """ branch_list = [] + two_months_ago = datetime.datetime.now() - datetime.timedelta(60) for repo in self.repositories: repo_branches = {} @@ -166,9 +170,16 @@ def get_repo_branches_involved(self, feature_id, hierarchy_regexes): # we haven't already done so and store them in the # temporary ``repo_branches`` dict if repo_branches[sha].get('commits') is None: - repo_branches[sha]['commits'] = [ - c.sha for c in repo.iter_commits(sha=sha) - ] + c_list = [] + for commit in repo.iter_commits(sha=sha): + commit_date = commit.commit.committer['date'] + commit_date_time = iso8601.parse_date( + commit_date) + if (commit_date_time.replace(tzinfo=None) < + two_months_ago): + break + c_list.append(commit.sha) + repo_branches[sha]['commits'] = c_list # Check if we're merged in parent_data = repo_branches[parent] has_parent = parent_data['sha'] in branch_data['commits'] diff --git a/deploystream/providers/jira/__init__.py b/deploystream/providers/jira/__init__.py index 4627170..c101800 100644 --- a/deploystream/providers/jira/__init__.py +++ b/deploystream/providers/jira/__init__.py @@ -34,6 +34,7 @@ def __init__(self, user, password, url, issue_types=None): if not issue_types: issue_types = ['Story', 'Bug'] self.issue_types = issue_types + self.base_url = url options = {'server': url} self._conn = JIRA(options, basic_auth=(user, password)) @@ -78,5 +79,27 @@ def get_feature_info(self, feature_id): ``None`` otherwise """ - raise NotImplementedError("get_feature_info() not implemented in {0}" - .format(self.__class__.__name__)) + jira_feature = self._conn.issue(feature_id) + if jira_feature: + return _transform(jira_feature) + # # url = '{0}/browse/{1}'.format( + # # self.base_url, jira_feature.key) + # # feature = { + # # 'title': jira_feature.fields.summary, + # # 'id': jira_feature.key, + # # 'url': url, + # # 'is_blocked': False, + # # 'type': jira_feature.fields.issuetype.name, + # # 'owner': '-', + # # } + + # # if jira_feature.fields: + # # if jira_feature.fields.assignee: + # # feature['owner'] = ( + # # jira_feature.fields.assignee.displayName) + + # # if jira_feature.fields.customfield_10200: + # # feature['legacy_id'] = ( + # # jira_feature.fields.customfield_10200) + + # return feature diff --git a/requirements/runtime.txt b/requirements/runtime.txt index 59abf03..d4a1fac 100644 --- a/requirements/runtime.txt +++ b/requirements/runtime.txt @@ -7,3 +7,4 @@ zope.interface # define and enforce interfaces certifi # Module for Mozilla's CA bundle apitopy # Required to implement the sprint.ly client jira-python>=0.13 # Required to access JIRA api +iso8601 # Parsing iso8601 datetimes. diff --git a/tests/test_feature/test_view.py b/tests/test_feature/test_view.py index fc2619b..4e7a902 100644 --- a/tests/test_feature/test_view.py +++ b/tests/test_feature/test_view.py @@ -82,7 +82,6 @@ def setUp(self): 'testbuild': BuildInfoProvider}) def test_feature_view_shows_details(self): response = self.client.get('/features/plan/FT101') - print response.__dict__ feature_dict = json.loads(response.data) assert_true("Amazing feature that will blow your mind" in feature_dict['title']) From baa5c9f36e86b81a8150120506adce4fa7ef72b2 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Mon, 27 May 2013 16:06:06 +0100 Subject: [PATCH 3/4] remove comments. --- deploystream/providers/jira/__init__.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/deploystream/providers/jira/__init__.py b/deploystream/providers/jira/__init__.py index c101800..a94ee79 100644 --- a/deploystream/providers/jira/__init__.py +++ b/deploystream/providers/jira/__init__.py @@ -82,24 +82,3 @@ def get_feature_info(self, feature_id): jira_feature = self._conn.issue(feature_id) if jira_feature: return _transform(jira_feature) - # # url = '{0}/browse/{1}'.format( - # # self.base_url, jira_feature.key) - # # feature = { - # # 'title': jira_feature.fields.summary, - # # 'id': jira_feature.key, - # # 'url': url, - # # 'is_blocked': False, - # # 'type': jira_feature.fields.issuetype.name, - # # 'owner': '-', - # # } - - # # if jira_feature.fields: - # # if jira_feature.fields.assignee: - # # feature['owner'] = ( - # # jira_feature.fields.assignee.displayName) - - # # if jira_feature.fields.customfield_10200: - # # feature['legacy_id'] = ( - # # jira_feature.fields.customfield_10200) - - # return feature From 0b6b0c1c84bb76306631351a609efa69422a8c74 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Mon, 27 May 2013 16:11:58 +0100 Subject: [PATCH 4/4] Amend tests. --- tests/test_providers/test_github_provider.py | 21 ++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/test_providers/test_github_provider.py b/tests/test_providers/test_github_provider.py index 9cddb95..6c44568 100644 --- a/tests/test_providers/test_github_provider.py +++ b/tests/test_providers/test_github_provider.py @@ -1,3 +1,4 @@ +import datetime from mock import Mock, patch from nose.tools import assert_equal, assert_true @@ -7,11 +8,19 @@ from tests import DEFAULT_HIERARCHY_REGEXES +def mock_commit(sha): + mock_comm = Mock(sha=sha) + mock_comm.commit.committer = {'date': datetime.datetime.now().isoformat()} + return mock_comm + + def mock_github3(github3): mock_repo = Mock() mock_repo.has_issues = True mock_repo.name = 'repo_1' - mock_repo.iter_commits.return_value = [Mock(sha="CoMmItHaSh-MaStEr")] + mock_repo.iter_commits.return_value = [ + mock_commit(sha="CoMmItHaSh-MaStEr") + ] issue1 = { 'title': 'Hello', @@ -36,15 +45,15 @@ def mock_github3(github3): branch1 = { 'name': 'master', - 'commit': Mock(sha='CoMmItHaSh-MaStEr'), + 'commit': mock_commit(sha='CoMmItHaSh-MaStEr'), } branch2 = { 'name': 'story/5/alex', - 'commit': Mock(sha='CoMmItHaSh-5'), + 'commit': mock_commit(sha='CoMmItHaSh-5'), } branch3 = { 'name': 'story/23/alex', - 'commit': Mock(sha='CoMmItHaSh-23'), + 'commit': mock_commit(sha='CoMmItHaSh-23'), } mock_branch1, mock_branch2, mock_branch3 = Mock(), Mock(), Mock() mock_branch1.__dict__ = branch1 @@ -82,8 +91,8 @@ def test_implements_expected_interfaces(_): def test_get_repo_branches_involved(github3): mock_github3(github3) github_provider = GithubProvider('token') - branches = github_provider.get_repo_branches_involved("5", - DEFAULT_HIERARCHY_REGEXES) + branches = github_provider.get_repo_branches_involved( + "5", DEFAULT_HIERARCHY_REGEXES) assert_equal(2, len(branches)) assert_true({ "repository": "repo_1",