Merge ~pappacena/launchpad:ui-ocirecipe-args into launchpad:master
- Git
- lp:~pappacena/launchpad
- ui-ocirecipe-args
- Merge into master
Proposed by
Thiago F. Pappacena
Status: | Merged |
---|---|
Approved by: | Thiago F. Pappacena |
Approved revision: | 04c9ed5f8bab13f5c0485f6d41fa38607248c295 |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~pappacena/launchpad:ui-ocirecipe-args |
Merge into: | launchpad:master |
Prerequisite: | ~pappacena/launchpad:ocirecipe-args |
Diff against target: |
302 lines (+166/-6) 4 files modified
lib/lp/oci/browser/ocirecipe.py (+59/-6) lib/lp/oci/browser/tests/test_ocirecipe.py (+92/-0) lib/lp/oci/templates/ocirecipe-index.pt (+12/-0) lib/lp/oci/templates/ocirecipe-new.pt (+3/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+389931@code.launchpad.net |
This proposal supersedes a proposal from 2020-08-27.
Commit message
UI to manage build ARGs for OCI recipes
Description of the change
To post a comment you must log in.
- 6919ec2... by Thiago F. Pappacena
-
Merge branch 'ocirecipe-args' into ui-ocirecipe-args
Revision history for this message
Thiago F. Pappacena (pappacena) wrote : | # |
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Approve
- 7c3074c... by Thiago F. Pappacena
-
Merge branch 'ocirecipe-args' into ui-ocirecipe-args
Revision history for this message
Tom Wardill (twom) : | # |
- 0c1d56d... by Thiago F. Pappacena
-
Small adjustments on build ARG presentation
Revision history for this message
Thiago F. Pappacena (pappacena) : | # |
Revision history for this message
Colin Watson (cjwatson) : | # |
Revision history for this message
Colin Watson (cjwatson) : | # |
- 2fb9015... by Thiago F. Pappacena
-
Fixing tag placement
- 04c9ed5... by Thiago F. Pappacena
-
Merge branch 'ocirecipe-args' into ui-ocirecipe-args
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py |
2 | index 5e6a50f..cf11bfb 100644 |
3 | --- a/lib/lp/oci/browser/ocirecipe.py |
4 | +++ b/lib/lp/oci/browser/ocirecipe.py |
5 | @@ -23,9 +23,12 @@ from lazr.restful.interface import ( |
6 | copy_field, |
7 | use_template, |
8 | ) |
9 | +import six |
10 | from zope.component import getUtility |
11 | from zope.formlib.form import FormFields |
12 | +from zope.formlib.textwidgets import TextAreaWidget |
13 | from zope.formlib.widget import ( |
14 | + CustomWidgetFactory, |
15 | DisplayWidget, |
16 | renderElement, |
17 | ) |
18 | @@ -35,6 +38,7 @@ from zope.schema import ( |
19 | Choice, |
20 | List, |
21 | Password, |
22 | + Text, |
23 | TextLine, |
24 | ValidationError, |
25 | ) |
26 | @@ -242,6 +246,12 @@ class OCIRecipeView(LaunchpadView): |
27 | else: |
28 | return "Built on request" |
29 | |
30 | + @property |
31 | + def build_args(self): |
32 | + return "\n".join( |
33 | + "%s=%s" % (k, v) |
34 | + for k, v in sorted(self.context.build_args.items())) |
35 | + |
36 | |
37 | def builds_for_recipe(recipe): |
38 | """A list of interesting builds. |
39 | @@ -676,6 +686,7 @@ class IOCIRecipeEditSchema(Interface): |
40 | "description", |
41 | "git_ref", |
42 | "build_file", |
43 | + "build_args", |
44 | "build_path", |
45 | "build_daily", |
46 | "require_virtualized", |
47 | @@ -683,7 +694,45 @@ class IOCIRecipeEditSchema(Interface): |
48 | ]) |
49 | |
50 | |
51 | -class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin): |
52 | +class OCIRecipeFormMixin: |
53 | + """Mixin with common processing for both edit and add views.""" |
54 | + custom_widget_build_args = CustomWidgetFactory( |
55 | + TextAreaWidget, height=5, width=100) |
56 | + |
57 | + def createBuildArgsField(self): |
58 | + """Create a form field for OCIRecipe.build_args attribute.""" |
59 | + if IOCIRecipe.providedBy(self.context): |
60 | + default = "\n".join( |
61 | + "%s=%s" % (k, v) |
62 | + for k, v in sorted(self.context.build_args.items())) |
63 | + else: |
64 | + default = "" |
65 | + return FormFields(Text( |
66 | + __name__='build_args', |
67 | + title=u'Build-time ARG variables', |
68 | + description=("One per line. Each ARG should be in the format " |
69 | + "of ARG_KEY=arg_value."), |
70 | + default=default, |
71 | + required=False, readonly=False)) |
72 | + |
73 | + def validateBuildArgs(self, data): |
74 | + field_value = data.get('build_args') |
75 | + if not field_value: |
76 | + return |
77 | + build_args = {} |
78 | + for i, line in enumerate(field_value.split("\n")): |
79 | + if '=' not in line: |
80 | + msg = ("'%s' at line %s is not a valid KEY=value pair." % |
81 | + (line, i + 1)) |
82 | + self.setFieldError("build_args", six.text_type(msg)) |
83 | + return |
84 | + k, v = line.split('=', 1) |
85 | + build_args[k] = v |
86 | + data['build_args'] = build_args |
87 | + |
88 | + |
89 | +class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin, |
90 | + OCIRecipeFormMixin): |
91 | """View for creating OCI recipes.""" |
92 | |
93 | page_title = label = "Create a new OCI recipe" |
94 | @@ -708,6 +757,7 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin): |
95 | def setUpFields(self): |
96 | """See `LaunchpadFormView`.""" |
97 | super(OCIRecipeAddView, self).setUpFields() |
98 | + self.form_fields += self.createBuildArgsField() |
99 | self.form_fields += self.createEnabledProcessors( |
100 | getUtility(IProcessorSet).getAll(), |
101 | "The architectures that this OCI recipe builds for. Some " |
102 | @@ -748,15 +798,16 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin): |
103 | "There is already an OCI recipe owned by %s in %s with " |
104 | "this name." % ( |
105 | owner.display_name, self.context.display_name)) |
106 | + self.validateBuildArgs(data) |
107 | |
108 | @action("Create OCI recipe", name="create") |
109 | def create_action(self, action, data): |
110 | recipe = getUtility(IOCIRecipeSet).new( |
111 | name=data["name"], registrant=self.user, owner=data["owner"], |
112 | oci_project=self.context, git_ref=data["git_ref"], |
113 | - build_file=data["build_file"], build_path=data["build_path"], |
114 | - description=data["description"], |
115 | - build_daily=data["build_daily"], processors=data["processors"]) |
116 | + build_file=data["build_file"], description=data["description"], |
117 | + build_daily=data["build_daily"], build_args=data["build_args"], |
118 | + build_path=data["build_path"], processors=data["processors"]) |
119 | self.next_url = canonical_url(recipe) |
120 | |
121 | |
122 | @@ -798,7 +849,8 @@ class OCIRecipeAdminView(BaseOCIRecipeEditView): |
123 | field_names = ("require_virtualized", "allow_internet") |
124 | |
125 | |
126 | -class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin): |
127 | +class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin, |
128 | + OCIRecipeFormMixin): |
129 | """View for editing OCI recipes.""" |
130 | |
131 | @property |
132 | @@ -821,6 +873,7 @@ class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin): |
133 | def setUpFields(self): |
134 | """See `LaunchpadFormView`.""" |
135 | super(OCIRecipeEditView, self).setUpFields() |
136 | + self.form_fields += self.createBuildArgsField() |
137 | self.form_fields += self.createEnabledProcessors( |
138 | self.context.available_processors, |
139 | "The architectures that this OCI recipe builds for. Some " |
140 | @@ -860,7 +913,7 @@ class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin): |
141 | # This processor is restricted and currently |
142 | # enabled. Leave it untouched. |
143 | data["processors"].append(processor) |
144 | - |
145 | + self.validateBuildArgs(data) |
146 | |
147 | class OCIRecipeDeleteView(BaseOCIRecipeEditView): |
148 | """View for deleting OCI recipes.""" |
149 | diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py |
150 | index c854d14..9a3f0ea 100644 |
151 | --- a/lib/lp/oci/browser/tests/test_ocirecipe.py |
152 | +++ b/lib/lp/oci/browser/tests/test_ocirecipe.py |
153 | @@ -215,6 +215,26 @@ class TestOCIRecipeAddView(BaseTestOCIRecipeView): |
154 | "Build schedule:\nBuilt on request\nEdit OCI recipe\n", |
155 | MatchesTagText(content, "build-schedule")) |
156 | |
157 | + def test_create_new_recipe_with_build_args(self): |
158 | + oci_project = self.factory.makeOCIProject() |
159 | + [git_ref] = self.factory.makeGitRefs() |
160 | + browser = self.getViewBrowser( |
161 | + oci_project, view_name="+new-recipe", user=self.person) |
162 | + browser.getControl(name="field.name").value = "recipe-name" |
163 | + browser.getControl("Description").value = "Recipe description" |
164 | + browser.getControl("Git repository").value = ( |
165 | + git_ref.repository.identity) |
166 | + browser.getControl("Git branch").value = git_ref.path |
167 | + browser.getControl("Build-time ARG variables").value = ( |
168 | + "VAR1=10\nVAR2=20") |
169 | + browser.getControl("Create OCI recipe").click() |
170 | + |
171 | + content = find_main_content(browser.contents) |
172 | + self.assertEqual("recipe-name", extract_text(content.h1)) |
173 | + self.assertThat( |
174 | + "Build-time\nARG variables:\nVAR1=10\nVAR2=20", |
175 | + MatchesTagText(content, "build-args")) |
176 | + |
177 | def test_create_new_recipe_users_teams_as_owner_options(self): |
178 | # Teams that the user is in are options for the OCI recipe owner. |
179 | self.factory.makeTeam( |
180 | @@ -466,6 +486,47 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView): |
181 | login_person(self.person) |
182 | self.assertRecipeProcessors(recipe, ["386", "amd64"]) |
183 | |
184 | + def test_edit_build_args(self): |
185 | + self.setUpDistroSeries() |
186 | + oci_project = self.factory.makeOCIProject(pillar=self.distribution) |
187 | + recipe = self.factory.makeOCIRecipe( |
188 | + registrant=self.person, owner=self.person, |
189 | + oci_project=oci_project, build_args={"VAR1": "xxx", "VAR2": "uu"}) |
190 | + browser = self.getViewBrowser( |
191 | + recipe, view_name="+edit", user=recipe.owner) |
192 | + args = browser.getControl(name="field.build_args") |
193 | + self.assertContentEqual("VAR1=xxx\r\nVAR2=uu", args.value) |
194 | + args.value = "VAR=aa\nANOTHER_VAR=bbb" |
195 | + browser.getControl("Update OCI recipe").click() |
196 | + login_person(self.person) |
197 | + IStore(recipe).reload(recipe) |
198 | + self.assertEqual( |
199 | + {"VAR": "aa", "ANOTHER_VAR": "bbb"}, recipe.build_args) |
200 | + |
201 | + def test_edit_build_args_invalid_content(self): |
202 | + self.setUpDistroSeries() |
203 | + oci_project = self.factory.makeOCIProject(pillar=self.distribution) |
204 | + recipe = self.factory.makeOCIRecipe( |
205 | + registrant=self.person, owner=self.person, |
206 | + oci_project=oci_project, build_args={"VAR1": "xxx", "VAR2": "uu"}) |
207 | + browser = self.getViewBrowser( |
208 | + recipe, view_name="+edit", user=recipe.owner) |
209 | + args = browser.getControl(name="field.build_args") |
210 | + self.assertContentEqual("VAR1=xxx\r\nVAR2=uu", args.value) |
211 | + args.value = "VAR=aa\nmessed up text" |
212 | + browser.getControl("Update OCI recipe").click() |
213 | + |
214 | + # Error message should be shown. |
215 | + content = find_main_content(browser.contents) |
216 | + self.assertIn( |
217 | + "'messed up text' at line 2 is not a valid KEY=value pair.", |
218 | + extract_text(content)) |
219 | + |
220 | + # Assert that recipe still have the original build_args. |
221 | + login_person(self.person) |
222 | + IStore(recipe).reload(recipe) |
223 | + self.assertEqual({"VAR1": "xxx", "VAR2": "uu"}, recipe.build_args) |
224 | + |
225 | def test_edit_with_invisible_processor(self): |
226 | # It's possible for existing recipes to have an enabled processor |
227 | # that's no longer usable with the current distroseries, which will |
228 | @@ -705,6 +766,37 @@ class TestOCIRecipeView(BaseTestOCIRecipeView): |
229 | """ % (oci_project_name, oci_project_display), |
230 | self.getMainText(build.recipe)) |
231 | |
232 | + def test_index_with_build_args(self): |
233 | + oci_project = self.factory.makeOCIProject( |
234 | + pillar=self.distroseries.distribution) |
235 | + oci_project_name = oci_project.name |
236 | + oci_project_display = oci_project.display_name |
237 | + [ref] = self.factory.makeGitRefs( |
238 | + owner=self.person, target=self.person, name="recipe-repository", |
239 | + paths=["refs/heads/master"]) |
240 | + recipe = self.makeOCIRecipe( |
241 | + oci_project=oci_project, git_ref=ref, build_file="Dockerfile", |
242 | + build_args={"VAR1": "123", "VAR2": "XXX"}) |
243 | + build = self.makeBuild( |
244 | + recipe=recipe, status=BuildStatus.FULLYBUILT, |
245 | + duration=timedelta(minutes=30)) |
246 | + self.assertTextMatchesExpressionIgnoreWhitespace("""\ |
247 | + %s OCI project |
248 | + recipe-name |
249 | + .* |
250 | + OCI recipe information |
251 | + Owner: Test Person |
252 | + OCI project: %s |
253 | + Source: ~test-person/\\+git/recipe-repository:master |
254 | + Build file path: Dockerfile |
255 | + Build schedule: Built on request |
256 | + Build-time\nARG variables: VAR1=123 VAR2=XXX |
257 | + Latest builds |
258 | + Status When complete Architecture |
259 | + Successfully built 30 minutes ago 386 |
260 | + """ % (oci_project_name, oci_project_display), |
261 | + self.getMainText(build.recipe)) |
262 | + |
263 | def test_index_success_with_buildlog(self): |
264 | # The build log is shown if it is there. |
265 | build = self.makeBuild( |
266 | diff --git a/lib/lp/oci/templates/ocirecipe-index.pt b/lib/lp/oci/templates/ocirecipe-index.pt |
267 | index c2ef69e..5c3ca56 100644 |
268 | --- a/lib/lp/oci/templates/ocirecipe-index.pt |
269 | +++ b/lib/lp/oci/templates/ocirecipe-index.pt |
270 | @@ -69,6 +69,18 @@ |
271 | <a tal:replace="structure view/menu:overview/edit/fmt:icon"/> |
272 | </dd> |
273 | </dl> |
274 | + <dl id="build-args" |
275 | + tal:define="build_args view/build_args" |
276 | + tal:condition="build_args"> |
277 | + <dt> |
278 | + Build-time |
279 | + <a href="https://docs.docker.com/engine/reference/commandline/build/#set-build-time-variables---build-arg" |
280 | + target="_blank">ARG variables</a>: |
281 | + </dt> |
282 | + <dd> |
283 | + <pre tal:content="build_args" /> |
284 | + </dd> |
285 | + </dl> |
286 | </div> |
287 | |
288 | <h2>Latest builds</h2> |
289 | diff --git a/lib/lp/oci/templates/ocirecipe-new.pt b/lib/lp/oci/templates/ocirecipe-new.pt |
290 | index 0bc156a..1cae71e 100644 |
291 | --- a/lib/lp/oci/templates/ocirecipe-new.pt |
292 | +++ b/lib/lp/oci/templates/ocirecipe-new.pt |
293 | @@ -35,6 +35,9 @@ |
294 | <tal:widget define="widget nocall:view/widgets/build_daily"> |
295 | <metal:block use-macro="context/@@launchpad_form/widget_row" /> |
296 | </tal:widget> |
297 | + <tal:widget define="widget nocall:view/widgets/build_args"> |
298 | + <metal:block use-macro="context/@@launchpad_form/widget_row" /> |
299 | + </tal:widget> |
300 | <tal:widget define="widget nocall:view/widgets/processors"> |
301 | <metal:block use-macro="context/@@launchpad_form/widget_row" /> |
302 | </tal:widget> |
I forgot to add the screenshots for this. I'll do it in some minutes.