Merge ~sylvain-pineau/checkbox-ng:submission-description into checkbox-ng:master

Proposed by Sylvain Pineau
Status: Merged
Approved by: Sylvain Pineau
Approved revision: 94a4b08d7dad93cbb586bc9b9d78e711dcbb7e71
Merged at revision: 7e275c4d221dbc4203f286cdbaa2cbaf888b9841
Proposed branch: ~sylvain-pineau/checkbox-ng:submission-description
Merge into: checkbox-ng:master
Diff against target: 282 lines (+55/-17)
7 files modified
checkbox_ng/launcher/subcommands.py (+27/-3)
plainbox/impl/exporter/jinja2.py (+7/-0)
plainbox/impl/exporter/test_jinja2.py (+2/-0)
plainbox/impl/providers/exporters/data/checkbox.json (+5/-0)
plainbox/impl/session/state.py (+1/-1)
plainbox/impl/session/test_state.py (+1/-1)
plainbox/impl/session/test_suspend.py (+12/-12)
Reviewer Review Type Date Requested Status
Maciej Kisielewski Approve
Sheila Miguez (community) Approve
Sylvain Pineau (community) Needs Resubmitting
Review via email: mp+340125@code.launchpad.net

Description of the change

Add support for a submission description field.
A new -m parameter can now be used to pass a multiline var, (with both checkbox-cli launcher|submit) e.g:

$ checkbox-cli submit xxxxxxxxxxxxxx submission.tar.xz -m "sdfsdf
sfsdfg set se t

serzret

https://askubuntu.com" -s

Or using a launcher:

$ ./canonical-certification-cli -m "foo bar"

Tested on staging: https://certification.staging.canonical.com/hardware/201202-10584/submission/119616/

To post a comment you must log in.
Revision history for this message
Maciej Kisielewski (kissiel) wrote :

Couple of (non-blocking) things inline I'd recommend changing.

review: Abstain
Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

Few replies inline, I'm reworking app_blob

Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

Reworked version with app_blob defaulting to b''.

review: Needs Resubmitting
Revision history for this message
Sheila Miguez (codersquid) wrote :

LGTM from me

review: Approve
Revision history for this message
Maciej Kisielewski (kissiel) wrote :

Great stuff, +1.
Also thanks for the explanation!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/checkbox_ng/launcher/subcommands.py b/checkbox_ng/launcher/subcommands.py
2index ae2b9b8..00205a9 100644
3--- a/checkbox_ng/launcher/subcommands.py
4+++ b/checkbox_ng/launcher/subcommands.py
5@@ -18,8 +18,10 @@
6 """
7 Definition of sub-command classes for checkbox-cli
8 """
9+from argparse import ArgumentTypeError
10 from argparse import SUPPRESS
11 from collections import defaultdict
12+from tempfile import TemporaryDirectory
13 import copy
14 import datetime
15 import fnmatch
16@@ -30,6 +32,7 @@ import operator
17 import os
18 import re
19 import sys
20+import tarfile
21 import time
22
23 from guacamole import Command
24@@ -89,22 +92,39 @@ class Submit(Command):
25 parser.add_argument(
26 "-s", "--staging", action="store_true",
27 help=_("Use staging environment"))
28+ parser.add_argument(
29+ "-m", "--message",
30+ help=_("Submission description"))
31
32 def invoked(self, ctx):
33 transport_cls = None
34- enc = None
35 mode = 'rb'
36 options_string = "secure_id={0}".format(ctx.args.secure_id)
37 url = ('https://certification.canonical.com/'
38 'api/v1/submission/{}/'.format(ctx.args.secure_id))
39+ submission_file = ctx.args.submission
40 if ctx.args.staging:
41 url = ('https://certification.staging.canonical.com/'
42 'api/v1/submission/{}/'.format(ctx.args.secure_id))
43 from checkbox_ng.certification import SubmissionServiceTransport
44 transport_cls = SubmissionServiceTransport
45 transport = transport_cls(url, options_string)
46+ if ctx.args.message:
47+ tmpdir = TemporaryDirectory()
48+ with tarfile.open(ctx.args.submission) as tar:
49+ tar.extractall(tmpdir.name)
50+ with open(os.path.join(tmpdir.name, 'submission.json')) as f:
51+ json_payload = json.load(f)
52+ with open(os.path.join(tmpdir.name, 'submission.json'), 'w') as f:
53+ json_payload['description'] = ctx.args.message
54+ json.dump(json_payload, f, sort_keys=True, indent=4)
55+ new_subm_file = os.path.join(
56+ tmpdir.name, os.path.basename(ctx.args.submission))
57+ with tarfile.open(new_subm_file, mode='w:xz') as tar:
58+ tar.add(tmpdir.name, arcname='')
59+ submission_file = new_subm_file
60 try:
61- with open(ctx.args.submission, mode, encoding=enc) as subm_file:
62+ with open(submission_file, mode) as subm_file:
63 result = transport.send(subm_file)
64 except (TransportError, OSError) as exc:
65 raise SystemExit(exc)
66@@ -377,7 +397,8 @@ class Launcher(Command, MainLoopStage):
67 raise SystemExit(_("No test plan selected."))
68 self.ctx.sa.select_test_plan(tp_id)
69 self.ctx.sa.update_app_blob(json.dumps(
70- {'testplan_id': tp_id, }).encode("UTF-8"))
71+ {'testplan_id': tp_id,
72+ 'description': self.ctx.args.message, }).encode("UTF-8"))
73 bs_jobs = self.ctx.sa.get_bootstrap_todo_list()
74 self._run_bootstrap_jobs(bs_jobs)
75 self.ctx.sa.finish_bootstrap()
76@@ -785,6 +806,9 @@ class Launcher(Command, MainLoopStage):
77 '--title', action='store', metavar='SESSION_NAME',
78 help=_('title of the session to use'))
79 parser.add_argument(
80+ "-m", "--message",
81+ help=_("submission description"))
82+ parser.add_argument(
83 '--dont-suppress-output', action='store_true', default=False,
84 help=_('Absolutely always show command output'))
85 # the next to options are and should be exact copies of what the
86diff --git a/plainbox/impl/exporter/jinja2.py b/plainbox/impl/exporter/jinja2.py
87index 8bced61..37ad825 100644
88--- a/plainbox/impl/exporter/jinja2.py
89+++ b/plainbox/impl/exporter/jinja2.py
90@@ -152,11 +152,18 @@ class Jinja2SessionStateExporter(ISessionStateExporter):
91 Byte stream to write to.
92
93 """
94+ try:
95+ state = session_manager.state
96+ app_blob = state.metadata.app_blob
97+ app_blob_data = json.loads(app_blob.decode("UTF-8"))
98+ except ValueError:
99+ app_blob_data = {}
100 data = {
101 'OUTCOME_METADATA_MAP': OUTCOME_METADATA_MAP,
102 'client_name': self._client_name,
103 'client_version': self._client_version,
104 'manager': session_manager,
105+ 'app_blob': app_blob_data,
106 'options': self.option_list,
107 'system_id': self._system_id,
108 'timestamp': self._timestamp,
109diff --git a/plainbox/impl/exporter/test_jinja2.py b/plainbox/impl/exporter/test_jinja2.py
110index d35455d..6ae5ce0 100644
111--- a/plainbox/impl/exporter/test_jinja2.py
112+++ b/plainbox/impl/exporter/test_jinja2.py
113@@ -31,6 +31,7 @@ import os
114
115 from plainbox.impl.exporter.jinja2 import Jinja2SessionStateExporter
116 from plainbox.impl.result import MemoryJobResult
117+from plainbox.impl.session.state import SessionMetaData
118 from plainbox.impl.unit.exporter import ExporterUnitSupport
119 from plainbox.impl.unit.job import JobDefinition
120 from plainbox.vendor import mock
121@@ -47,6 +48,7 @@ class Jinja2SessionStateExporterTests(TestCase):
122 job = mock.Mock(spec_set=JobDefinition, id='job_id')
123 job.tr_summary.return_value = 'job name'
124 self.manager_single_job = mock.Mock(state=mock.Mock(
125+ metadata=SessionMetaData(),
126 run_list=[job],
127 job_state_map={
128 job.id: mock.Mock(result=result, job=job)
129diff --git a/plainbox/impl/providers/exporters/data/checkbox.json b/plainbox/impl/providers/exporters/data/checkbox.json
130index cd49b05..4257bca 100644
131--- a/plainbox/impl/providers/exporters/data/checkbox.json
132+++ b/plainbox/impl/providers/exporters/data/checkbox.json
133@@ -3,6 +3,11 @@
134 {%- set resource_map = state.resource_map -%}
135 {%- set job_state_map = state.job_state_map -%}
136 {
137+{%- if "description" in app_blob %}
138+{%- if app_blob['description'] %}
139+ "description": {{ app_blob['description'] | jsonify | safe }},
140+{%- endif %}
141+{%- endif %}
142 {%- if ns ~ 'package' in state.resource_map %}
143 "packages": [
144 {%- set package_resource_list = state.resource_map[ns ~ 'package'] %}
145diff --git a/plainbox/impl/session/state.py b/plainbox/impl/session/state.py
146index 825137c..9993689 100644
147--- a/plainbox/impl/session/state.py
148+++ b/plainbox/impl/session/state.py
149@@ -75,7 +75,7 @@ class SessionMetaData:
150 FLAG_TESTPLANLESS = "testplanless"
151
152 def __init__(self, title=None, flags=None, running_job_name=None,
153- app_blob=None, app_id=None):
154+ app_blob=b'', app_id=None):
155 """Initialize a new session state meta-data object."""
156 if flags is None:
157 flags = []
158diff --git a/plainbox/impl/session/test_state.py b/plainbox/impl/session/test_state.py
159index fcb93f7..83fe78f 100644
160--- a/plainbox/impl/session/test_state.py
161+++ b/plainbox/impl/session/test_state.py
162@@ -802,7 +802,7 @@ class SessionMetadataTests(TestCase):
163
164 def test_app_blob_default_value(self):
165 metadata = SessionMetaData()
166- self.assertIs(metadata.app_blob, None)
167+ self.assertIs(metadata.app_blob, b'')
168
169 def test_app_blob_assignment(self):
170 metadata = SessionMetaData()
171diff --git a/plainbox/impl/session/test_suspend.py b/plainbox/impl/session/test_suspend.py
172index a2f2aad..ab4c6ca 100644
173--- a/plainbox/impl/session/test_suspend.py
174+++ b/plainbox/impl/session/test_suspend.py
175@@ -367,7 +367,7 @@ class SessionSuspendHelper2Tests(SessionSuspendHelper1Tests):
176 'title': None,
177 'flags': [],
178 'running_job_name': None,
179- 'app_blob': None
180+ 'app_blob': ''
181 })
182
183 def test_repr_SessionMetaData_typical_metadata(self):
184@@ -403,7 +403,7 @@ class SessionSuspendHelper2Tests(SessionSuspendHelper1Tests):
185 'title': None,
186 'flags': [],
187 'running_job_name': None,
188- 'app_blob': None,
189+ 'app_blob': '',
190 },
191 })
192
193@@ -427,7 +427,7 @@ class SessionSuspendHelper2Tests(SessionSuspendHelper1Tests):
194 self.assertEqual(gzip.decompress(data), (
195 b'{"session":{"desired_job_list":[],"jobs":{},'
196 b'"mandatory_job_list":[],"metadata":'
197- b'{"app_blob":null,"flags":[],"running_job_name":null,"title":null'
198+ b'{"app_blob":"","flags":[],"running_job_name":null,"title":null'
199 b'},"results":{}},"version":2}'))
200
201
202@@ -458,7 +458,7 @@ class SessionSuspendHelper3Tests(SessionSuspendHelper2Tests):
203 'title': None,
204 'flags': [],
205 'running_job_name': None,
206- 'app_blob': None,
207+ 'app_blob': '',
208 'app_id': None
209 })
210
211@@ -497,7 +497,7 @@ class SessionSuspendHelper3Tests(SessionSuspendHelper2Tests):
212 'title': None,
213 'flags': [],
214 'running_job_name': None,
215- 'app_blob': None,
216+ 'app_blob': '',
217 'app_id': None,
218 },
219 })
220@@ -521,7 +521,7 @@ class SessionSuspendHelper3Tests(SessionSuspendHelper2Tests):
221 self.assertEqual(gzip.decompress(data), (
222 b'{"session":{"desired_job_list":[],"jobs":{},'
223 b'"mandatory_job_list":[],"metadata":'
224- b'{"app_blob":null,"app_id":null,"flags":[],'
225+ b'{"app_blob":"","app_id":null,"flags":[],'
226 b'"running_job_name":null,"title":null},"results":{}},'
227 b'"version":3}'))
228
229@@ -590,7 +590,7 @@ class SessionSuspendHelper4Tests(SessionSuspendHelper3Tests):
230 'title': None,
231 'flags': [],
232 'running_job_name': None,
233- 'app_blob': None,
234+ 'app_blob': '',
235 'app_id': None,
236 },
237 })
238@@ -614,7 +614,7 @@ class SessionSuspendHelper4Tests(SessionSuspendHelper3Tests):
239 self.assertEqual(gzip.decompress(data), (
240 b'{"session":{"desired_job_list":[],"jobs":{},'
241 b'"mandatory_job_list":[],"metadata":'
242- b'{"app_blob":null,"app_id":null,"flags":[],'
243+ b'{"app_blob":"","app_id":null,"flags":[],'
244 b'"running_job_name":null,"title":null},"results":{}},'
245 b'"version":4}'))
246
247@@ -654,7 +654,7 @@ class SessionSuspendHelper5Tests(SessionSuspendHelper4Tests):
248 self.assertEqual(gzip.decompress(data), (
249 b'{"session":{"desired_job_list":[],"jobs":{},'
250 b'"mandatory_job_list":[],"metadata":'
251- b'{"app_blob":null,"app_id":null,"flags":[],'
252+ b'{"app_blob":"","app_id":null,"flags":[],'
253 b'"running_job_name":null,"title":null},"results":{}},'
254 b'"version":5}'))
255
256@@ -694,7 +694,7 @@ class SessionSuspendHelper6Tests(SessionSuspendHelper5Tests):
257 self.assertEqual(gzip.decompress(data), (
258 b'{"session":{"desired_job_list":[],"jobs":{},'
259 b'"mandatory_job_list":[],"metadata":'
260- b'{"app_blob":null,"app_id":null,"flags":[],'
261+ b'{"app_blob":"","app_id":null,"flags":[],'
262 b'"running_job_name":null,"title":null},"results":{}},'
263 b'"version":6}'))
264
265@@ -746,7 +746,7 @@ class SessionSuspendHelper6Tests(SessionSuspendHelper5Tests):
266 'title': None,
267 'flags': [],
268 'running_job_name': None,
269- 'app_blob': None,
270+ 'app_blob': '',
271 'app_id': None,
272 },
273 })
274@@ -766,7 +766,7 @@ class SessionSuspendHelper6Tests(SessionSuspendHelper5Tests):
275 'title': None,
276 'flags': [],
277 'running_job_name': None,
278- 'app_blob': None,
279+ 'app_blob': '',
280 'app_id': None,
281 },
282 })

Subscribers

People subscribed via source and target branches