Launchpad itself

Merge lp:~wgrant/launchpad/no-lucilleconfig-directories into lp:launchpad

  1. no-lucilleconfig-directories
  2. Merge into devel
Proposed by William Grant
Status: Merged
Approved by: William Grant
Approved revision: no longer in the source branch.
Merged at revision: 12108
Proposed branch: lp:~wgrant/launchpad/no-lucilleconfig-directories
Merge into: lp:launchpad
Prerequisite: lp:~wgrant/launchpad/destroy-distroseries-lucilleconfig
Diff against target: 475 lines (+319/-50)
6 files modified
lib/lp/archivepublisher/config.py (+29/-44)
lib/lp/archivepublisher/tests/test_ftparchive.py (+2/-2)
lib/lp/archivepublisher/tests/test_publisher.py (+2/-2)
lib/lp/code/interfaces/branchmergequeue.py (+129/-0)
lib/lp/code/model/tests/test_branchmergequeue.py (+155/-0)
lib/lp/soyuz/tests/test_publishing.py (+2/-2)
To merge this branch: bzr merge lp:~wgrant/launchpad/no-lucilleconfig-directories
Reviewer Review Type Date Requested Status
Tim Penhey (community) mentor Approve
Steve Kowalik (community) code* Approve
Review via email: mp+38650@code.launchpad.net

Commit message

[r=stevenk][ui=none][bug=691784] Calculate archive publication paths from the LAZR config, not lucilleconfig.

Description of the change

A continuation of my anti-lucilleconfig tirade from lp:~wgrant/launchpad/destroy-distroseries-lucilleconfig, this branch obsoletes Distribution.lucilleconfig's "publishing" section.

getPubConfig has conditionally overridden Config's paths since PPAs were introduced some years ago. This branch moves the calculation out of Config and into getPubConfig, replacing dependency on in-database configuration with values that are already in the LAZR config.

Before this can land, production configs need to be checked. The only change should be that ${archivepublisher.root}/ubuntu-test is now used instead of /srv/launchpad.net/ubuntu-archive/ubuntu-test, but this may make germanium unhappy.

To post a comment you must log in.
Revision history for this message
Steve Kowalik (stevenk) wrote :

Hi William,

Firstly, thank you for the clean up. My two concerns are:

 * Why are you adding another variable APT_FTPARCHIVE_PURPOSES? It's there one that already encompasses that from lp.soyuz.interfaces.archive?
 * Do any other callsites utilitise the code you've changed and therefore their tests need fixing too?

review: Needs Information (code*)
Revision history for this message
Steve Kowalik (stevenk) wrote :

I was mistaken, there isn't a variable that encompasses PRIMARY and DEBUG in lp.soyuz.interfaces.archive. I think there should be, since there is likely other places the code that could benefit from it.

Revision history for this message
Steve Kowalik (stevenk) wrote :

After discussions with William, my first objection is withdrawn, your honour.

review: Approve (code*)
Revision history for this message
Tim Penhey (thumper) :
review: Approve (mentor)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/archivepublisher/config.py'
2--- lib/lp/archivepublisher/config.py 2010-10-15 08:06:41 +0000
3+++ lib/lp/archivepublisher/config.py 2010-12-18 00:21:49 +0000
4@@ -14,16 +14,7 @@
5 from lp.soyuz.enums import ArchivePurpose
6
7
8-def update_pub_config(pubconf):
9- """Update dependent `PubConfig` fields.
10-
11- Update fields dependending on 'archiveroot'.
12- """
13- pubconf.poolroot = os.path.join(pubconf.archiveroot, 'pool')
14- pubconf.distsroot = os.path.join(pubconf.archiveroot, 'dists')
15- pubconf.overrideroot = None
16- pubconf.cacheroot = None
17- pubconf.miscroot = None
18+APT_FTPARCHIVE_PURPOSES = (ArchivePurpose.PRIMARY, ArchivePurpose.COPY)
19
20
21 def getPubConfig(archive):
22@@ -36,9 +27,10 @@
23 pubconf = Config(archive.distribution)
24 ppa_config = config.personalpackagearchive
25
26- if archive.purpose == ArchivePurpose.PRIMARY:
27- pass
28- elif archive.is_ppa:
29+ pubconf.temproot = os.path.join(
30+ config.archivepublisher.root, '%s-temp' % pubconf.distroName)
31+
32+ if archive.is_ppa:
33 if archive.private:
34 pubconf.distroroot = ppa_config.private_root
35 pubconf.htaccessroot = os.path.join(
36@@ -49,34 +41,41 @@
37 pubconf.archiveroot = os.path.join(
38 pubconf.distroroot, archive.owner.name, archive.name,
39 archive.distribution.name)
40- update_pub_config(pubconf)
41- elif archive.purpose == ArchivePurpose.PARTNER:
42- pubconf.distroroot = config.archivepublisher.root
43- pubconf.archiveroot = os.path.join(
44- pubconf.distroroot, archive.distribution.name + '-partner')
45- update_pub_config(pubconf)
46- elif archive.purpose == ArchivePurpose.DEBUG:
47- pubconf.distroroot = config.archivepublisher.root
48- pubconf.archiveroot = os.path.join(
49- pubconf.distroroot, archive.distribution.name + '-debug')
50- update_pub_config(pubconf)
51+ elif archive.is_main:
52+ pubconf.distroroot = config.archivepublisher.root
53+ pubconf.archiveroot = os.path.join(
54+ pubconf.distroroot, archive.distribution.name)
55+ if archive.purpose == ArchivePurpose.PARTNER:
56+ pubconf.archiveroot += '-partner'
57+ elif archive.purpose == ArchivePurpose.DEBUG:
58+ pubconf.archiveroot += '-debug'
59 elif archive.is_copy:
60 pubconf.distroroot = config.archivepublisher.root
61 pubconf.archiveroot = os.path.join(
62 pubconf.distroroot,
63 archive.distribution.name + '-' + archive.name,
64 archive.distribution.name)
65- # Multiple copy archives can exist on the same machine so the
66- # temp areas need to be unique also.
67+ else:
68+ raise AssertionError(
69+ "Unknown archive purpose %s when getting publisher config.",
70+ archive.purpose)
71+
72+ # There can be multiple copy archives, so the temp dir needs to be
73+ # within the archive.
74+ if archive.is_copy:
75 pubconf.temproot = pubconf.archiveroot + '-temp'
76- update_pub_config(pubconf)
77+
78+ if archive.purpose in APT_FTPARCHIVE_PURPOSES:
79 pubconf.overrideroot = pubconf.archiveroot + '-overrides'
80 pubconf.cacheroot = pubconf.archiveroot + '-cache'
81 pubconf.miscroot = pubconf.archiveroot + '-misc'
82 else:
83- raise AssertionError(
84- "Unknown archive purpose %s when getting publisher config.",
85- archive.purpose)
86+ pubconf.overrideroot = None
87+ pubconf.cacheroot = None
88+ pubconf.miscroot = None
89+
90+ pubconf.poolroot = os.path.join(pubconf.archiveroot, 'pool')
91+ pubconf.distsroot = os.path.join(pubconf.archiveroot, 'dists')
92
93 meta_root = os.path.join(
94 pubconf.distroroot, archive.owner.name)
95@@ -140,20 +139,6 @@
96 self.stayofexecution = self._distroconfig.get(
97 "publishing", "pendingremovalduration", 5)
98 self.stayofexecution = float(self.stayofexecution)
99- self.distroroot = self._distroconfig.get("publishing","root")
100- self.archiveroot = self._distroconfig.get("publishing","archiveroot")
101- self.poolroot = self._distroconfig.get("publishing","poolroot")
102- self.distsroot = self._distroconfig.get("publishing","distsroot")
103- self.overrideroot = self._distroconfig.get(
104- "publishing","overrideroot")
105- self.cacheroot = self._distroconfig.get("publishing","cacheroot")
106- self.miscroot = self._distroconfig.get("publishing","miscroot")
107- # XXX cprov 2007-04-26 bug=45270:
108- # We should build all the previous attributes
109- # dynamically like this. It would reduce the configuration complexity.
110- # Even before we have it properly modeled in LPDB.
111- self.temproot = os.path.join(
112- self.distroroot, '%s-temp' % self.distroName)
113
114 def setupArchiveDirs(self):
115 """Create missing required directories in archive.
116
117=== modified file 'lib/lp/archivepublisher/tests/test_ftparchive.py'
118--- lib/lp/archivepublisher/tests/test_ftparchive.py 2010-10-17 09:03:43 +0000
119+++ lib/lp/archivepublisher/tests/test_ftparchive.py 2010-12-18 00:21:49 +0000
120@@ -20,7 +20,7 @@
121 QuietFakeLogger,
122 )
123 from canonical.testing.layers import LaunchpadZopelessLayer
124-from lp.archivepublisher.config import Config
125+from lp.archivepublisher.config import getPubConfig
126 from lp.archivepublisher.diskpool import DiskPool
127 from lp.archivepublisher.ftparchive import (
128 f_touch,
129@@ -71,7 +71,7 @@
130
131 self._distribution = getUtility(IDistributionSet)['ubuntutest']
132 self._archive = self._distribution.main_archive
133- self._config = Config(self._distribution)
134+ self._config = getPubConfig(self._archive)
135 self._config.setupArchiveDirs()
136 self._sampledir = os.path.join(
137 config.root, "lib", "lp", "archivepublisher", "tests",
138
139=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
140--- lib/lp/archivepublisher/tests/test_publisher.py 2010-12-02 15:35:38 +0000
141+++ lib/lp/archivepublisher/tests/test_publisher.py 2010-12-18 00:21:49 +0000
142@@ -1142,7 +1142,7 @@
143 self.ubuntutest.getSeries('breezy-autotest').status = (
144 SeriesStatus.FROZEN)
145
146- self.config = Config(self.ubuntutest)
147+ self.config = getPubConfig(self.ubuntutest.main_archive)
148 publisher = Publisher(
149 self.logger, self.config, self.disk_pool,
150 self.ubuntutest.main_archive)
151@@ -1163,7 +1163,7 @@
152
153 ds = self.ubuntutest.getSeries('breezy-autotest')
154 ds.getDistroArchSeries('i386').enabled = False
155- self.config = Config(self.ubuntutest)
156+ self.config = getPubConfig(self.ubuntutest.main_archive)
157
158 publisher = Publisher(
159 self.logger, self.config, self.disk_pool,
160
161=== added file 'lib/lp/code/interfaces/branchmergequeue.py'
162--- lib/lp/code/interfaces/branchmergequeue.py 1970-01-01 00:00:00 +0000
163+++ lib/lp/code/interfaces/branchmergequeue.py 2010-11-03 08:28:44 +0000
164@@ -0,0 +1,129 @@
165+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
166+# GNU Affero General Public License version 3 (see the file LICENSE).
167+
168+"""Branch merge queue interfaces."""
169+
170+__metaclass__ = type
171+
172+__all__ = [
173+ 'IBranchMergeQueue',
174+ 'IBranchMergeQueueSource',
175+ 'user_has_special_merge_queue_access',
176+ ]
177+
178+from lazr.restful.declarations import (
179+ export_as_webservice_entry,
180+ export_write_operation,
181+ exported,
182+ mutator_for,
183+ operation_parameters,
184+ )
185+from lazr.restful.fields import (
186+ CollectionField,
187+ Reference,
188+ )
189+from zope.component import getUtility
190+from zope.interface import Interface
191+from zope.schema import (
192+ Datetime,
193+ Int,
194+ Text,
195+ TextLine,
196+ )
197+
198+from canonical.launchpad import _
199+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
200+from lp.services.fields import (
201+ PersonChoice,
202+ PublicPersonChoice,
203+ )
204+
205+
206+class IBranchMergeQueue(Interface):
207+ """An interface for managing branch merges."""
208+
209+ export_as_webservice_entry()
210+
211+ id = Int(title=_('ID'), readonly=True, required=True)
212+
213+ registrant = exported(
214+ PublicPersonChoice(
215+ title=_("The user that registered the branch."),
216+ required=True, readonly=True,
217+ vocabulary='ValidPersonOrTeam'))
218+
219+ owner = exported(
220+ PersonChoice(
221+ title=_('Owner'),
222+ required=True, readonly=True,
223+ vocabulary='UserTeamsParticipationPlusSelf',
224+ description=_("The owner of the merge queue.")))
225+
226+ name = exported(
227+ TextLine(
228+ title=_('Name'), required=True,
229+ description=_(
230+ "Keep very short, unique, and descriptive, because it will "
231+ "be used in URLs. "
232+ "Examples: main, devel, release-1.0, gnome-vfs.")))
233+
234+ description = exported(
235+ Text(
236+ title=_('Description'), required=False,
237+ description=_(
238+ 'A short description of the purpose of this merge queue.')))
239+
240+ configuration = exported(
241+ TextLine(
242+ title=_('Configuration'), required=False, readonly=True,
243+ description=_(
244+ "A JSON string of configuration values.")))
245+
246+ date_created = exported(
247+ Datetime(
248+ title=_('Date Created'),
249+ required=True,
250+ readonly=True))
251+
252+ branches = exported(
253+ CollectionField(
254+ title=_('Dependent Branches'),
255+ description=_(
256+ 'A collection of branches that this queue manages.'),
257+ readonly=True,
258+ value_type=Reference(Interface)))
259+
260+ @mutator_for(configuration)
261+ @operation_parameters(
262+ config=TextLine(title=_("A JSON string of configuration values.")))
263+ @export_write_operation()
264+ def setMergeQueueConfig(config):
265+ """Set the JSON string configuration of the merge queue.
266+
267+ :param config: A JSON string of configuration values.
268+ """
269+
270+
271+class IBranchMergeQueueSource(Interface):
272+
273+ def new(name, owner, registrant, description, configuration, branches):
274+ """Create a new IBranchMergeQueue object.
275+
276+ :param name: The name of the branch merge queue.
277+ :param description: A description of queue.
278+ :param configuration: A JSON string of configuration values.
279+ :param owner: The owner of the queue.
280+ :param registrant: The registrant of the queue.
281+ :param branches: A list of branches to add to the queue.
282+ """
283+
284+
285+def user_has_special_merge_queue_access(user):
286+ """Admins and bazaar experts have special access.
287+
288+ :param user: A 'Person' or None.
289+ """
290+ if user is None:
291+ return False
292+ celebs = getUtility(ILaunchpadCelebrities)
293+ return user.inTeam(celebs.admin) or user.inTeam(celebs.bazaar_experts)
294
295=== added file 'lib/lp/code/model/tests/test_branchmergequeue.py'
296--- lib/lp/code/model/tests/test_branchmergequeue.py 1970-01-01 00:00:00 +0000
297+++ lib/lp/code/model/tests/test_branchmergequeue.py 2010-10-25 14:51:56 +0000
298@@ -0,0 +1,155 @@
299+# Copyright 2010 Canonical Ltd. This software is licensed under the
300+# GNU Affero General Public License version 3 (see the file LICENSE).
301+
302+"""Unit tests for methods of BranchMergeQueue."""
303+
304+from __future__ import with_statement
305+
306+import simplejson
307+
308+from canonical.launchpad.interfaces.lpstorm import IStore
309+from canonical.launchpad.webapp.testing import verifyObject
310+from canonical.testing.layers import (
311+ AppServerLayer,
312+ DatabaseFunctionalLayer,
313+ )
314+from lp.code.errors import InvalidMergeQueueConfig
315+from lp.code.interfaces.branchmergequeue import IBranchMergeQueue
316+from lp.code.model.branchmergequeue import BranchMergeQueue
317+from lp.testing import (
318+ ANONYMOUS,
319+ person_logged_in,
320+ launchpadlib_for,
321+ TestCaseWithFactory,
322+ ws_object,
323+ )
324+
325+
326+class TestBranchMergeQueueInterface(TestCaseWithFactory):
327+ """Test IBranchMergeQueue interface."""
328+
329+ layer = DatabaseFunctionalLayer
330+
331+ def test_implements_interface(self):
332+ queue = self.factory.makeBranchMergeQueue()
333+ IStore(BranchMergeQueue).add(queue)
334+ verifyObject(IBranchMergeQueue, queue)
335+
336+
337+class TestBranchMergeQueueSource(TestCaseWithFactory):
338+ """Test the methods of IBranchMergeQueueSource."""
339+
340+ layer = DatabaseFunctionalLayer
341+
342+ def test_new(self):
343+ owner = self.factory.makePerson()
344+ name = u'SooperQueue'
345+ description = u'This is Sooper Queue'
346+ config = unicode(simplejson.dumps({'test': 'make check'}))
347+
348+ queue = BranchMergeQueue.new(
349+ name, owner, owner, description, config)
350+
351+ self.assertEqual(queue.name, name)
352+ self.assertEqual(queue.owner, owner)
353+ self.assertEqual(queue.registrant, owner)
354+ self.assertEqual(queue.description, description)
355+ self.assertEqual(queue.configuration, config)
356+
357+
358+class TestBranchMergeQueue(TestCaseWithFactory):
359+ """Test the functions of the BranchMergeQueue."""
360+
361+ layer = DatabaseFunctionalLayer
362+
363+ def test_branches(self):
364+ """Test that a merge queue can get all its managed branches."""
365+ store = IStore(BranchMergeQueue)
366+
367+ queue = self.factory.makeBranchMergeQueue()
368+ store.add(queue)
369+
370+ branch = self.factory.makeBranch()
371+ store.add(branch)
372+ with person_logged_in(branch.owner):
373+ branch.addToQueue(queue)
374+
375+ self.assertEqual(
376+ list(queue.branches),
377+ [branch])
378+
379+ def test_setMergeQueueConfig(self):
380+ """Test that the configuration is set properly."""
381+ queue = self.factory.makeBranchMergeQueue()
382+ config = unicode(simplejson.dumps({
383+ 'test': 'make test'}))
384+
385+ with person_logged_in(queue.owner):
386+ queue.setMergeQueueConfig(config)
387+
388+ self.assertEqual(queue.configuration, config)
389+
390+ def test_setMergeQueueConfig_invalid_json(self):
391+ """Test that invalid json can't be set as the config."""
392+ queue = self.factory.makeBranchMergeQueue()
393+
394+ with person_logged_in(queue.owner):
395+ self.assertRaises(
396+ InvalidMergeQueueConfig,
397+ queue.setMergeQueueConfig,
398+ 'abc')
399+
400+
401+class TestWebservice(TestCaseWithFactory):
402+
403+ layer = AppServerLayer
404+
405+ def test_properties(self):
406+ """Test that the correct properties are exposed."""
407+ with person_logged_in(ANONYMOUS):
408+ name = u'teh-queue'
409+ description = u'Oh hai! I are a queues'
410+ configuration = unicode(simplejson.dumps({'test': 'make check'}))
411+
412+ queuer = self.factory.makePerson()
413+ db_queue = self.factory.makeBranchMergeQueue(
414+ registrant=queuer, owner=queuer, name=name,
415+ description=description,
416+ configuration=configuration)
417+ branch1 = self.factory.makeBranch()
418+ with person_logged_in(branch1.owner):
419+ branch1.addToQueue(db_queue)
420+ branch2 = self.factory.makeBranch()
421+ with person_logged_in(branch2.owner):
422+ branch2.addToQueue(db_queue)
423+ launchpad = launchpadlib_for('test', db_queue.owner,
424+ service_root="http://api.launchpad.dev:8085")
425+
426+ queuer = ws_object(launchpad, queuer)
427+ queue = ws_object(launchpad, db_queue)
428+ branch1 = ws_object(launchpad, branch1)
429+ branch2 = ws_object(launchpad, branch2)
430+
431+ self.assertEqual(queue.registrant, queuer)
432+ self.assertEqual(queue.owner, queuer)
433+ self.assertEqual(queue.name, name)
434+ self.assertEqual(queue.description, description)
435+ self.assertEqual(queue.configuration, configuration)
436+ self.assertEqual(queue.date_created, db_queue.date_created)
437+ self.assertEqual(len(queue.branches), 2)
438+
439+ def test_set_configuration(self):
440+ """Test the mutator for setting configuration."""
441+ with person_logged_in(ANONYMOUS):
442+ db_queue = self.factory.makeBranchMergeQueue()
443+ launchpad = launchpadlib_for('test', db_queue.owner,
444+ service_root="http://api.launchpad.dev:8085")
445+
446+ configuration = simplejson.dumps({'test': 'make check'})
447+
448+ queue = ws_object(launchpad, db_queue)
449+ queue.configuration = configuration
450+ queue.lp_save()
451+
452+ queue2 = ws_object(launchpad, db_queue)
453+ self.assertEqual(queue2.configuration, configuration)
454
455=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
456--- lib/lp/soyuz/tests/test_publishing.py 2010-12-15 06:03:14 +0000
457+++ lib/lp/soyuz/tests/test_publishing.py 2010-12-18 00:21:49 +0000
458@@ -26,7 +26,7 @@
459 )
460 from canonical.testing.layers import reconnect_stores
461 from lp.app.errors import NotFoundError
462-from lp.archivepublisher.config import Config
463+from lp.archivepublisher.config import getPubConfig
464 from lp.archivepublisher.diskpool import DiskPool
465 from lp.buildmaster.enums import BuildStatus
466 from lp.registry.interfaces.distribution import IDistributionSet
467@@ -571,7 +571,7 @@
468 super(TestNativePublishingBase, self).setUp()
469 self.layer.switchDbUser(config.archivepublisher.dbuser)
470 self.prepareBreezyAutotest()
471- self.config = Config(self.ubuntutest)
472+ self.config = getPubConfig(self.ubuntutest.main_archive)
473 self.config.setupArchiveDirs()
474 self.pool_dir = self.config.poolroot
475 self.temp_dir = self.config.temproot