Merge lp:~mars/tarmac/fix-plugin-import into lp:~launchpad/tarmac/lp-tarmac

Proposed by Māris Fogels on 2010-12-16
Status: Merged
Approved by: Māris Fogels on 2010-12-16
Approved revision: 384
Merged at revision: 385
Proposed branch: lp:~mars/tarmac/fix-plugin-import
Merge into: lp:~launchpad/tarmac/lp-tarmac
Diff against target: 112 lines (+43/-6)
3 files modified
tarmac/plugin.py (+5/-1)
tarmac/plugins/allowedcontributors.py (+23/-3)
tarmac/plugins/tests/test_allowedcontributors.py (+15/-2)
To merge this branch: bzr merge lp:~mars/tarmac/fix-plugin-import
Reviewer Review Type Date Requested Status
Māris Fogels (community) Approve on 2010-12-16
Review via email: mp+43853@code.launchpad.net

Commit message

Fix the Tarmac plugin activation code so plugins in .config are recognized.

Description of the change

Hi,

This branch presents a quick and dirty fix to the Tarmac plugin activation code. It adds the Tarmac plugin directories to the system path so that Tarmac can import the plugins.

There may be a better way to do this, but I am leaving that better implementation for someone else to tackle. The code passed manual testing on my local system.

Maris

To post a comment you must log in.
Māris Fogels (mars) wrote :

Self-approved for the Launchpad Tarmac trial and landing on the lp-tarmac fork.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tarmac/plugin.py'
2--- tarmac/plugin.py 2010-09-02 15:18:07 +0000
3+++ tarmac/plugin.py 2010-12-16 02:21:27 +0000
4@@ -18,6 +18,7 @@
5
6 import imp
7 import os
8+import sys
9
10 from tarmac import plugins as _mod_plugins
11
12@@ -40,6 +41,9 @@
13
14 plugin_names = set()
15 for path in TARMAC_PLUGIN_PATHS:
16+ if path not in sys.path:
17+ sys.path.insert(0, path)
18+
19 try:
20 for _file in os.listdir(path):
21 full_path = os.path.join(path, _file)
22@@ -75,6 +79,6 @@
23
24 for name in plugin_names:
25 try:
26- exec 'import tarmac.plugins.%s' % name in {}
27+ exec 'import %s' % name in {}
28 except KeyboardInterrupt:
29 raise
30
31=== modified file 'tarmac/plugins/allowedcontributors.py'
32--- tarmac/plugins/allowedcontributors.py 2010-09-20 18:25:54 +0000
33+++ tarmac/plugins/allowedcontributors.py 2010-12-16 02:21:27 +0000
34@@ -23,6 +23,10 @@
35 """Error for when a contributor does not meet validation requirements."""
36
37
38+class InvalidPersonOrTeam(TarmacMergeError):
39+ """Error for when a required team could not be found."""
40+
41+
42 class AllowedContributors(TarmacPlugin):
43 """Tarmac plug-in for checking whether a contributor is allowed to.
44
45@@ -52,9 +56,25 @@
46 in_team = False
47 for team in self.allowed_contributors:
48 launchpad = command.launchpad
49- lp_members = launchpad.people[team].getMembersByStatus(
50- status=[u'Approved', u'Administrator'])
51- members = [x.name for x in lp_members]
52+ try:
53+ lp_members = launchpad.people[team].getMembersByStatus(
54+ status=u'Approved')
55+ lp_members.extend(
56+ launchpad.people[team].getMembersByStatus(
57+ status=u'Administrator'))
58+ members = [x.name for x in lp_members]
59+ except KeyError:
60+ message = (u'Could not find person or team "%s" on '
61+ u'Launchpad.' % team)
62+ comment = (u'Merging into %(target) requires that '
63+ u'contributing authors be a member of an '
64+ u'acceptable team, or a specified person. '
65+ u'However, the person or team "%(team)s" '
66+ u'was not found on Launchpad.' % {
67+ 'target': proposal.target_branch.display_name,
68+ 'team': team})
69+ raise InvalidPersonOrTeam(message, comment)
70+
71 if author in members:
72 in_team = True
73 break
74
75=== modified file 'tarmac/plugins/tests/test_allowedcontributors.py'
76--- tarmac/plugins/tests/test_allowedcontributors.py 2010-09-20 18:25:54 +0000
77+++ tarmac/plugins/tests/test_allowedcontributors.py 2010-12-16 02:21:27 +0000
78@@ -16,7 +16,7 @@
79 """Tests for the Allowed Contributors plug-in."""
80
81 from tarmac.plugins.allowedcontributors import (
82- InvalidContributor, AllowedContributors)
83+ InvalidContributor, InvalidPersonOrTeam, AllowedContributors)
84 from tarmac.tests import TarmacTestCase
85 from tarmac.tests.mock import Thing
86
87@@ -47,7 +47,7 @@
88 Thing(name=u'person3', status=u'Approved'),
89 Thing(name=u'person8', status=u'Invited'),
90 Thing(name=u'personX', status=u'Expired')]
91- return [x for x in members if x.status in status]
92+ return [x for x in members if x.status == status]
93
94 def noMembersByStatus(self, status=None):
95 """Fake method to not return any members for people."""
96@@ -74,3 +74,16 @@
97 self.plugin.run,
98 command=command, target=target, source=source,
99 proposal=self.proposal)
100+
101+ def test_run_private_team(self):
102+ """Test that the plug-in runs correctly."""
103+ config = Thing(allowed_contributors=u'private_team')
104+ source = Thing(authors=[u'person1', u'person2', u'person3'])
105+ target = Thing(config=config)
106+ launchpad = Thing(people=self.people)
107+ command = Thing(launchpad=launchpad)
108+ self.assertRaises(InvalidPersonOrTeam,
109+ self.plugin.run,
110+ command=command, target=target, source=source,
111+ proposal=self.proposal)
112+

Subscribers

People subscribed via source and target branches

to all changes: