Merge lp:~cjwatson/launchpad/snap-build-channels-ui into lp:launchpad

Proposed by Colin Watson on 2018-04-30
Status: Needs review
Proposed branch: lp:~cjwatson/launchpad/snap-build-channels-ui
Merge into: lp:launchpad
Diff against target: 511 lines (+322/-3)
10 files modified
lib/lp/snappy/browser/snap.py (+26/-2)
lib/lp/snappy/browser/tests/test_snap.py (+16/-0)
lib/lp/snappy/browser/widgets/snapbuildchannels.py (+99/-0)
lib/lp/snappy/browser/widgets/templates/snapbuildchannels.pt (+16/-0)
lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py (+132/-0)
lib/lp/snappy/model/snapbuildbehaviour.py (+2/-1)
lib/lp/snappy/templates/snap-edit.pt (+3/-0)
lib/lp/snappy/templates/snap-index.pt (+15/-0)
lib/lp/snappy/templates/snap-new.pt (+3/-0)
lib/lp/snappy/tests/test_snapbuildbehaviour.py (+10/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/snap-build-channels-ui
Reviewer Review Type Date Requested Status
Launchpad code reviewers 2018-04-30 Pending
Review via email: mp+344859@code.launchpad.net

Commit message

Add UI for Snap.auto_build_channels.

Description of the change

In the process of doing this work, it became clear that we're going to need some kind of escape syntax for continuing to install snapcraft using apt in order to make the migration work. There's more rationale for my choice there in the relevant commit message.

To post a comment you must log in.

Unmerged revisions

18627. By Colin Watson on 2018-04-30

Add UI for Snap.auto_build_channels.

18626. By Colin Watson on 2018-04-30

Special-case auto_build_channels={"snapcraft": "apt"} to install snapcraft using apt.

Without something like this, it's hard to do a well-managed migration to
installing snapcraft from the stable channel by default. We're going to
need a way for individual snaps to opt out in case they don't work with
snapcraft as a snap for whatever reason, and if the unmarked default case
(i.e. `auto_build_channels=None`) becomes equivalent to `{"snapcraft":
"stable"}` then they need some other escape route.

The alternative would be to do a data migration to set `{"snapcraft":
"stable"}` for everything, but that's hard to revert if something goes wrong
and isn't in the spirit of feature flags.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/snappy/browser/snap.py'
2--- lib/lp/snappy/browser/snap.py 2018-04-21 10:01:22 +0000
3+++ lib/lp/snappy/browser/snap.py 2018-04-30 16:52:16 +0000
4@@ -80,6 +80,9 @@
5 from lp.services.webapp.url import urlappend
6 from lp.services.webhooks.browser import WebhookTargetNavigationMixin
7 from lp.snappy.browser.widgets.snaparchive import SnapArchiveWidget
8+from lp.snappy.browser.widgets.snapbuildchannels import (
9+ SnapBuildChannelsWidget,
10+ )
11 from lp.snappy.browser.widgets.storechannels import StoreChannelsWidget
12 from lp.snappy.interfaces.snap import (
13 CannotAuthorizeStoreUploads,
14@@ -190,6 +193,12 @@
15 return 'Built on request'
16
17 @property
18+ def sorted_auto_build_channels_items(self):
19+ if self.context.auto_build_channels is None:
20+ return []
21+ return sorted(self.context.auto_build_channels.items())
22+
23+ @property
24 def store_channels(self):
25 return ', '.join(self.context.store_channels)
26
27@@ -322,8 +331,16 @@
28 'allow_internet',
29 'build_source_tarball',
30 'auto_build',
31+ 'auto_build_channels',
32 'store_upload',
33 ])
34+ auto_build_channels = copy_field(
35+ ISnap['auto_build_channels'],
36+ description=(
37+ u'The channels to use for build tools when building the snap '
38+ u'package. If unset, or if the channel for snapcraft is set to '
39+ u'"apt", the default behaviour is to install snapcraft from the '
40+ u'source archive using apt.'))
41 store_distro_series = Choice(
42 vocabulary='BuildableSnappyDistroSeries', required=True,
43 title=u'Series')
44@@ -379,14 +396,16 @@
45 'auto_build',
46 'auto_build_archive',
47 'auto_build_pocket',
48+ 'auto_build_channels',
49 'store_upload',
50 'store_name',
51 'store_channels',
52 ]
53 custom_widget('store_distro_series', LaunchpadRadioWidget)
54 custom_widget('auto_build_archive', SnapArchiveWidget)
55+ custom_widget('auto_build_pocket', LaunchpadDropdownWidget)
56+ custom_widget('auto_build_channels', SnapBuildChannelsWidget)
57 custom_widget('store_channels', StoreChannelsWidget)
58- custom_widget('auto_build_pocket', LaunchpadDropdownWidget)
59
60 help_links = {
61 "auto_build_pocket": u"/+help-snappy/snap-build-pocket.html",
62@@ -512,6 +531,7 @@
63 auto_build=data['auto_build'],
64 auto_build_archive=data['auto_build_archive'],
65 auto_build_pocket=data['auto_build_pocket'],
66+ auto_build_channels=data.get('auto_build_channels'),
67 processors=data['processors'], private=private,
68 build_source_tarball=data['build_source_tarball'],
69 store_upload=data['store_upload'],
70@@ -624,6 +644,8 @@
71 del data['auto_build_archive']
72 if 'auto_build_pocket' in data:
73 del data['auto_build_pocket']
74+ if 'auto_build_channels' in data:
75+ del data['auto_build_channels']
76 store_upload = data.get('store_upload', False)
77 if not store_upload:
78 if 'store_name' in data:
79@@ -688,16 +710,18 @@
80 'auto_build',
81 'auto_build_archive',
82 'auto_build_pocket',
83+ 'auto_build_channels',
84 'store_upload',
85 'store_name',
86 'store_channels',
87 ]
88 custom_widget('store_distro_series', LaunchpadRadioWidget)
89- custom_widget('store_channels', StoreChannelsWidget)
90 custom_widget('vcs', LaunchpadRadioWidget)
91 custom_widget('git_ref', GitRefWidget, allow_external=True)
92 custom_widget('auto_build_archive', SnapArchiveWidget)
93 custom_widget('auto_build_pocket', LaunchpadDropdownWidget)
94+ custom_widget('auto_build_channels', SnapBuildChannelsWidget)
95+ custom_widget('store_channels', StoreChannelsWidget)
96
97 help_links = {
98 "auto_build_pocket": u"/+help-snappy/snap-build-pocket.html",
99
100=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
101--- lib/lp/snappy/browser/tests/test_snap.py 2018-04-21 10:15:26 +0000
102+++ lib/lp/snappy/browser/tests/test_snap.py 2018-04-30 16:52:16 +0000
103@@ -254,6 +254,7 @@
104 self.assertThat(
105 "Pocket for automatic builds:\n\nEdit snap package",
106 MatchesTagText(content, "auto_build_pocket"))
107+ self.assertIsNone(find_tag_by_id(content, "auto_build_channels"))
108 self.assertThat(
109 "Builds of this snap package are not automatically uploaded to "
110 "the store.\nEdit snap package",
111@@ -291,6 +292,7 @@
112 self.assertThat(
113 "Pocket for automatic builds:\n\nEdit snap package",
114 MatchesTagText(content, "auto_build_pocket"))
115+ self.assertIsNone(find_tag_by_id(content, "auto_build_channels"))
116 self.assertThat(
117 "Builds of this snap package are not automatically uploaded to "
118 "the store.\nEdit snap package",
119@@ -389,6 +391,10 @@
120 browser.getControl(name="field.auto_build_archive.ppa").value = (
121 archive.reference)
122 browser.getControl("Pocket for automatic builds").value = ["SECURITY"]
123+ browser.getControl(
124+ name="field.auto_build_channels.core").value = "stable"
125+ browser.getControl(
126+ name="field.auto_build_channels.snapcraft").value = "edge"
127 browser.getControl("Create snap package").click()
128
129 content = find_main_content(browser.contents)
130@@ -402,6 +408,10 @@
131 self.assertThat(
132 "Pocket for automatic builds:\nSecurity\nEdit snap package",
133 MatchesTagText(content, "auto_build_pocket"))
134+ self.assertThat(
135+ "Source snap channels for automatic builds:\nEdit snap package\n"
136+ "core\nstable\nsnapcraft\nedge\n",
137+ MatchesTagText(content, "auto_build_channels"))
138
139 def test_create_new_snap_store_upload(self):
140 # Creating a new snap and asking for it to be automatically uploaded
141@@ -713,6 +723,8 @@
142 browser.getControl(name="field.auto_build_archive.ppa").value = (
143 archive.reference)
144 browser.getControl("Pocket for automatic builds").value = ["SECURITY"]
145+ browser.getControl(
146+ name="field.auto_build_channels.snapcraft").value = "edge"
147 browser.getControl("Update snap package").click()
148
149 content = find_main_content(browser.contents)
150@@ -739,6 +751,10 @@
151 "Pocket for automatic builds:\nSecurity\nEdit snap package",
152 MatchesTagText(content, "auto_build_pocket"))
153 self.assertThat(
154+ "Source snap channels for automatic builds:\nEdit snap package\n"
155+ "snapcraft\nedge",
156+ MatchesTagText(content, "auto_build_channels"))
157+ self.assertThat(
158 "Builds of this snap package are not automatically uploaded to "
159 "the store.\nEdit snap package",
160 MatchesTagText(content, "store_upload"))
161
162=== added file 'lib/lp/snappy/browser/widgets/snapbuildchannels.py'
163--- lib/lp/snappy/browser/widgets/snapbuildchannels.py 1970-01-01 00:00:00 +0000
164+++ lib/lp/snappy/browser/widgets/snapbuildchannels.py 2018-04-30 16:52:16 +0000
165@@ -0,0 +1,99 @@
166+# Copyright 2018 Canonical Ltd. This software is licensed under the
167+# GNU Affero General Public License version 3 (see the file LICENSE).
168+
169+"""XXX: Module docstring goes here."""
170+
171+from __future__ import absolute_import, print_function, unicode_literals
172+
173+__metaclass__ = type
174+__all__ = [
175+ 'SnapBuildChannelsWidget',
176+ ]
177+
178+from z3c.ptcompat import ViewPageTemplateFile
179+from zope.formlib.interfaces import IInputWidget
180+from zope.formlib.utility import setUpWidget
181+from zope.formlib.widget import (
182+ BrowserWidget,
183+ InputErrors,
184+ InputWidget,
185+ )
186+from zope.interface import implementer
187+from zope.schema import TextLine
188+from zope.security.proxy import isinstance as zope_isinstance
189+
190+from lp.app.errors import UnexpectedFormData
191+from lp.services.webapp.interfaces import (
192+ IAlwaysSubmittedWidget,
193+ ISingleLineWidgetLayout,
194+ )
195+
196+
197+@implementer(ISingleLineWidgetLayout, IAlwaysSubmittedWidget, IInputWidget)
198+class SnapBuildChannelsWidget(BrowserWidget, InputWidget):
199+
200+ template = ViewPageTemplateFile("templates/snapbuildchannels.pt")
201+ hint = False
202+ snap_names = ["core", "snapcraft"]
203+ _widgets_set_up = False
204+
205+ def setUpSubWidgets(self):
206+ if self._widgets_set_up:
207+ return
208+ fields = [
209+ TextLine(
210+ __name__=snap_name, title="%s channel" % snap_name,
211+ required=False)
212+ for snap_name in self.snap_names
213+ ]
214+ for field in fields:
215+ setUpWidget(
216+ self, field.__name__, field, IInputWidget, prefix=self.name)
217+ self._widgets_set_up = True
218+
219+ def setRenderedValue(self, value):
220+ """See `IWidget`."""
221+ self.setUpSubWidgets()
222+ if not zope_isinstance(value, dict):
223+ value = {}
224+ self.core_widget.setRenderedValue(value.get("core"))
225+ self.snapcraft_widget.setRenderedValue(value.get("snapcraft"))
226+
227+ def hasInput(self):
228+ """See `IInputWidget`."""
229+ return any(
230+ "%s.%s" % (self.name, snap_name) in self.request.form
231+ for snap_name in self.snap_names)
232+
233+ def hasValidInput(self):
234+ """See `IInputWidget`."""
235+ try:
236+ self.getInputValue()
237+ return True
238+ except (InputErrors, UnexpectedFormData):
239+ return False
240+
241+ def getInputValue(self):
242+ """See `IInputWidget`."""
243+ self.setUpSubWidgets()
244+ channels = {}
245+ for snap_name in self.snap_names:
246+ widget = getattr(self, snap_name + "_widget")
247+ channel = widget.getInputValue()
248+ if channel:
249+ channels[snap_name] = channel
250+ return channels
251+
252+ def error(self):
253+ """See `IBrowserWidget`."""
254+ try:
255+ if self.hasInput():
256+ self.getInputValue()
257+ except InputErrors as error:
258+ self._error = error
259+ return super(SnapBuildChannelsWidget, self).error()
260+
261+ def __call__(self):
262+ """See `IBrowserWidget`."""
263+ self.setUpSubWidgets()
264+ return self.template()
265
266=== added file 'lib/lp/snappy/browser/widgets/templates/snapbuildchannels.pt'
267--- lib/lp/snappy/browser/widgets/templates/snapbuildchannels.pt 1970-01-01 00:00:00 +0000
268+++ lib/lp/snappy/browser/widgets/templates/snapbuildchannels.pt 2018-04-30 16:52:16 +0000
269@@ -0,0 +1,16 @@
270+<tal:root
271+ xmlns:tal="http://xml.zope.org/namespaces/tal"
272+ omit-tag="">
273+
274+<table class="subordinate">
275+ <tr>
276+ <td>core</td>
277+ <td><div tal:content="structure view/core_widget" /></td>
278+ </tr>
279+ <tr>
280+ <td>snapcraft</td>
281+ <td><div tal:content="structure view/snapcraft_widget" /></td>
282+ </tr>
283+</table>
284+
285+</tal:root>
286
287=== added file 'lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py'
288--- lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py 1970-01-01 00:00:00 +0000
289+++ lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py 2018-04-30 16:52:16 +0000
290@@ -0,0 +1,132 @@
291+# Copyright 2018 Canonical Ltd. This software is licensed under the
292+# GNU Affero General Public License version 3 (see the file LICENSE).
293+
294+from __future__ import absolute_import, print_function, unicode_literals
295+
296+__metaclass__ = type
297+
298+import re
299+
300+from zope.formlib.interfaces import (
301+ IBrowserWidget,
302+ IInputWidget,
303+ )
304+from zope.schema import Dict
305+
306+from lp.services.beautifulsoup import BeautifulSoup
307+from lp.services.webapp.servers import LaunchpadTestRequest
308+from lp.snappy.browser.widgets.snapbuildchannels import (
309+ SnapBuildChannelsWidget,
310+ )
311+from lp.testing import (
312+ TestCaseWithFactory,
313+ verifyObject,
314+ )
315+from lp.testing.layers import DatabaseFunctionalLayer
316+
317+
318+class TestSnapBuildChannelsWidget(TestCaseWithFactory):
319+
320+ layer = DatabaseFunctionalLayer
321+
322+ def setUp(self):
323+ super(TestSnapBuildChannelsWidget, self).setUp()
324+ field = Dict(
325+ __name__="auto_build_channels",
326+ title="Source snap channels for automatic builds")
327+ self.context = self.factory.makeSnap()
328+ field = field.bind(self.context)
329+ request = LaunchpadTestRequest()
330+ self.widget = SnapBuildChannelsWidget(field, request)
331+
332+ def test_implements(self):
333+ self.assertTrue(verifyObject(IBrowserWidget, self.widget))
334+ self.assertTrue(verifyObject(IInputWidget, self.widget))
335+
336+ def test_template(self):
337+ self.assertTrue(
338+ self.widget.template.filename.endswith("snapbuildchannels.pt"),
339+ "Template was not set up.")
340+
341+ def test_setUpSubWidgets_first_call(self):
342+ # The subwidgets are set up and a flag is set.
343+ self.widget.setUpSubWidgets()
344+ self.assertTrue(self.widget._widgets_set_up)
345+ self.assertIsNotNone(getattr(self.widget, "core_widget", None))
346+ self.assertIsNotNone(getattr(self.widget, "snapcraft_widget", None))
347+
348+ def test_setUpSubWidgets_second_call(self):
349+ # The setUpSubWidgets method exits early if a flag is set to
350+ # indicate that the widgets were set up.
351+ self.widget._widgets_set_up = True
352+ self.widget.setUpSubWidgets()
353+ self.assertIsNone(getattr(self.widget, "core_widget", None))
354+ self.assertIsNone(getattr(self.widget, "snapcraft_widget", None))
355+
356+ def test_setRenderedValue_None(self):
357+ self.widget.setRenderedValue(None)
358+ self.assertIsNone(self.widget.core_widget._getCurrentValue())
359+ self.assertIsNone(self.widget.snapcraft_widget._getCurrentValue())
360+
361+ def test_setRenderedValue_empty(self):
362+ self.widget.setRenderedValue({})
363+ self.assertIsNone(self.widget.core_widget._getCurrentValue())
364+ self.assertIsNone(self.widget.snapcraft_widget._getCurrentValue())
365+
366+ def test_setRenderedValue_one_channel(self):
367+ self.widget.setRenderedValue({"snapcraft": "stable"})
368+ self.assertIsNone(self.widget.core_widget._getCurrentValue())
369+ self.assertEqual(
370+ "stable", self.widget.snapcraft_widget._getCurrentValue())
371+
372+ def test_setRenderedValue_all_channels(self):
373+ self.widget.setRenderedValue(
374+ {"core": "candidate", "snapcraft": "stable"})
375+ self.assertEqual(
376+ "candidate", self.widget.core_widget._getCurrentValue())
377+ self.assertEqual(
378+ "stable", self.widget.snapcraft_widget._getCurrentValue())
379+
380+ def test_hasInput_false(self):
381+ # hasInput is false when there are no channels in the form data.
382+ self.widget.request = LaunchpadTestRequest(form={})
383+ self.assertFalse(self.widget.hasInput())
384+
385+ def test_hasInput_true(self):
386+ # hasInput is true when there are channels in the form data.
387+ self.widget.request = LaunchpadTestRequest(
388+ form={"field.auto_build_channels.snapcraft": "stable"})
389+ self.assertTrue(self.widget.hasInput())
390+
391+ def test_hasValidInput_true(self):
392+ # The field input is valid when all submitted channels are valid.
393+ # (At the moment, individual channel names are not validated, so
394+ # there is no "false" counterpart to this test.)
395+ form = {
396+ "field.auto_build_channels.core": "",
397+ "field.auto_build_channels.snapcraft": "stable",
398+ }
399+ self.widget.request = LaunchpadTestRequest(form=form)
400+ self.assertTrue(self.widget.hasValidInput())
401+
402+ def test_getInputValue(self):
403+ form = {
404+ "field.auto_build_channels.core": "",
405+ "field.auto_build_channels.snapcraft": "stable",
406+ }
407+ self.widget.request = LaunchpadTestRequest(form=form)
408+ self.assertEqual({"snapcraft": "stable"}, self.widget.getInputValue())
409+
410+ def test_call(self):
411+ # The __call__ method sets up the widgets.
412+ markup = self.widget()
413+ self.assertIsNotNone(self.widget.core_widget)
414+ self.assertIsNotNone(self.widget.snapcraft_widget)
415+ soup = BeautifulSoup(markup)
416+ fields = soup.findAll(["input"], {"id": re.compile(".*")})
417+ expected_ids = [
418+ "field.auto_build_channels.core",
419+ "field.auto_build_channels.snapcraft",
420+ ]
421+ ids = [field["id"] for field in fields]
422+ self.assertContentEqual(expected_ids, ids)
423
424=== modified file 'lib/lp/snappy/model/snapbuildbehaviour.py'
425--- lib/lp/snappy/model/snapbuildbehaviour.py 2018-04-23 11:14:38 +0000
426+++ lib/lp/snappy/model/snapbuildbehaviour.py 2018-04-30 16:52:16 +0000
427@@ -110,7 +110,8 @@
428 logger=logger))
429 args["archive_private"] = build.archive.private
430 args["build_url"] = canonical_url(build)
431- if build.channels is not None:
432+ if (build.channels is not None and
433+ build.channels.get("snapcraft") != "apt"):
434 # We have to remove the security proxy that Zope applies to this
435 # dict, since otherwise we'll be unable to serialise it to
436 # XML-RPC.
437
438=== modified file 'lib/lp/snappy/templates/snap-edit.pt'
439--- lib/lp/snappy/templates/snap-edit.pt 2018-04-21 10:01:22 +0000
440+++ lib/lp/snappy/templates/snap-edit.pt 2018-04-30 16:52:16 +0000
441@@ -76,6 +76,9 @@
442 <tal:widget define="widget nocall:view/widgets/auto_build_pocket">
443 <metal:block use-macro="context/@@launchpad_form/widget_row" />
444 </tal:widget>
445+ <tal:widget define="widget nocall:view/widgets/auto_build_channels">
446+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
447+ </tal:widget>
448 </table>
449 </td>
450 </tr>
451
452=== modified file 'lib/lp/snappy/templates/snap-index.pt'
453--- lib/lp/snappy/templates/snap-index.pt 2018-04-21 10:15:26 +0000
454+++ lib/lp/snappy/templates/snap-index.pt 2018-04-30 16:52:16 +0000
455@@ -95,6 +95,21 @@
456 <a tal:replace="structure view/menu:overview/edit/fmt:icon"/>
457 </dd>
458 </dl>
459+ <dl id="auto_build_channels" tal:condition="context/auto_build_channels">
460+ <dt>
461+ Source snap channels for automatic builds:
462+ <a tal:replace="structure view/menu:overview/edit/fmt:icon"/>
463+ </dt>
464+ <dd>
465+ <table class="listing compressed">
466+ <tbody>
467+ <tr tal:repeat="pair view/sorted_auto_build_channels_items">
468+ <td tal:repeat="value pair" tal:content="value"/>
469+ </tr>
470+ </tbody>
471+ </table>
472+ </dd>
473+ </dl>
474 </div>
475
476 <div id="store_upload" class="two-column-list"
477
478=== modified file 'lib/lp/snappy/templates/snap-new.pt'
479--- lib/lp/snappy/templates/snap-new.pt 2018-04-21 10:01:22 +0000
480+++ lib/lp/snappy/templates/snap-new.pt 2018-04-30 16:52:16 +0000
481@@ -51,6 +51,9 @@
482 <tal:widget define="widget nocall:view/widgets/auto_build_pocket">
483 <metal:block use-macro="context/@@launchpad_form/widget_row" />
484 </tal:widget>
485+ <tal:widget define="widget nocall:view/widgets/auto_build_channels">
486+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
487+ </tal:widget>
488 </table>
489 </td>
490 </tr>
491
492=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
493--- lib/lp/snappy/tests/test_snapbuildbehaviour.py 2018-04-23 11:14:38 +0000
494+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py 2018-04-30 16:52:16 +0000
495@@ -428,6 +428,16 @@
496 self.assertEqual({"snapcraft": "edge"}, args["channels"])
497
498 @defer.inlineCallbacks
499+ def test_extraBuildArgs_channels_apt(self):
500+ # {"snapcraft": "apt"} causes snapcraft to be installed from apt.
501+ job = self.makeJob(channels={"snapcraft": "apt"})
502+ expected_archives, expected_trusted_keys = (
503+ yield get_sources_list_for_building(
504+ job.build, job.build.distro_arch_series, None))
505+ args = yield job._extraBuildArgs()
506+ self.assertNotIn("channels", args)
507+
508+ @defer.inlineCallbacks
509 def test_extraBuildArgs_disallow_internet(self):
510 # If external network access is not allowed for the snap,
511 # _extraBuildArgs does not dispatch a proxy token.