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 (community) 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
diff --git a/checkbox_ng/launcher/subcommands.py b/checkbox_ng/launcher/subcommands.py
index ae2b9b8..00205a9 100644
--- a/checkbox_ng/launcher/subcommands.py
+++ b/checkbox_ng/launcher/subcommands.py
@@ -18,8 +18,10 @@
18"""18"""
19Definition of sub-command classes for checkbox-cli19Definition of sub-command classes for checkbox-cli
20"""20"""
21from argparse import ArgumentTypeError
21from argparse import SUPPRESS22from argparse import SUPPRESS
22from collections import defaultdict23from collections import defaultdict
24from tempfile import TemporaryDirectory
23import copy25import copy
24import datetime26import datetime
25import fnmatch27import fnmatch
@@ -30,6 +32,7 @@ import operator
30import os32import os
31import re33import re
32import sys34import sys
35import tarfile
33import time36import time
3437
35from guacamole import Command38from guacamole import Command
@@ -89,22 +92,39 @@ class Submit(Command):
89 parser.add_argument(92 parser.add_argument(
90 "-s", "--staging", action="store_true",93 "-s", "--staging", action="store_true",
91 help=_("Use staging environment"))94 help=_("Use staging environment"))
95 parser.add_argument(
96 "-m", "--message",
97 help=_("Submission description"))
9298
93 def invoked(self, ctx):99 def invoked(self, ctx):
94 transport_cls = None100 transport_cls = None
95 enc = None
96 mode = 'rb'101 mode = 'rb'
97 options_string = "secure_id={0}".format(ctx.args.secure_id)102 options_string = "secure_id={0}".format(ctx.args.secure_id)
98 url = ('https://certification.canonical.com/'103 url = ('https://certification.canonical.com/'
99 'api/v1/submission/{}/'.format(ctx.args.secure_id))104 'api/v1/submission/{}/'.format(ctx.args.secure_id))
105 submission_file = ctx.args.submission
100 if ctx.args.staging:106 if ctx.args.staging:
101 url = ('https://certification.staging.canonical.com/'107 url = ('https://certification.staging.canonical.com/'
102 'api/v1/submission/{}/'.format(ctx.args.secure_id))108 'api/v1/submission/{}/'.format(ctx.args.secure_id))
103 from checkbox_ng.certification import SubmissionServiceTransport109 from checkbox_ng.certification import SubmissionServiceTransport
104 transport_cls = SubmissionServiceTransport110 transport_cls = SubmissionServiceTransport
105 transport = transport_cls(url, options_string)111 transport = transport_cls(url, options_string)
112 if ctx.args.message:
113 tmpdir = TemporaryDirectory()
114 with tarfile.open(ctx.args.submission) as tar:
115 tar.extractall(tmpdir.name)
116 with open(os.path.join(tmpdir.name, 'submission.json')) as f:
117 json_payload = json.load(f)
118 with open(os.path.join(tmpdir.name, 'submission.json'), 'w') as f:
119 json_payload['description'] = ctx.args.message
120 json.dump(json_payload, f, sort_keys=True, indent=4)
121 new_subm_file = os.path.join(
122 tmpdir.name, os.path.basename(ctx.args.submission))
123 with tarfile.open(new_subm_file, mode='w:xz') as tar:
124 tar.add(tmpdir.name, arcname='')
125 submission_file = new_subm_file
106 try:126 try:
107 with open(ctx.args.submission, mode, encoding=enc) as subm_file:127 with open(submission_file, mode) as subm_file:
108 result = transport.send(subm_file)128 result = transport.send(subm_file)
109 except (TransportError, OSError) as exc:129 except (TransportError, OSError) as exc:
110 raise SystemExit(exc)130 raise SystemExit(exc)
@@ -377,7 +397,8 @@ class Launcher(Command, MainLoopStage):
377 raise SystemExit(_("No test plan selected."))397 raise SystemExit(_("No test plan selected."))
378 self.ctx.sa.select_test_plan(tp_id)398 self.ctx.sa.select_test_plan(tp_id)
379 self.ctx.sa.update_app_blob(json.dumps(399 self.ctx.sa.update_app_blob(json.dumps(
380 {'testplan_id': tp_id, }).encode("UTF-8"))400 {'testplan_id': tp_id,
401 'description': self.ctx.args.message, }).encode("UTF-8"))
381 bs_jobs = self.ctx.sa.get_bootstrap_todo_list()402 bs_jobs = self.ctx.sa.get_bootstrap_todo_list()
382 self._run_bootstrap_jobs(bs_jobs)403 self._run_bootstrap_jobs(bs_jobs)
383 self.ctx.sa.finish_bootstrap()404 self.ctx.sa.finish_bootstrap()
@@ -785,6 +806,9 @@ class Launcher(Command, MainLoopStage):
785 '--title', action='store', metavar='SESSION_NAME',806 '--title', action='store', metavar='SESSION_NAME',
786 help=_('title of the session to use'))807 help=_('title of the session to use'))
787 parser.add_argument(808 parser.add_argument(
809 "-m", "--message",
810 help=_("submission description"))
811 parser.add_argument(
788 '--dont-suppress-output', action='store_true', default=False,812 '--dont-suppress-output', action='store_true', default=False,
789 help=_('Absolutely always show command output'))813 help=_('Absolutely always show command output'))
790 # the next to options are and should be exact copies of what the814 # the next to options are and should be exact copies of what the
diff --git a/plainbox/impl/exporter/jinja2.py b/plainbox/impl/exporter/jinja2.py
index 8bced61..37ad825 100644
--- a/plainbox/impl/exporter/jinja2.py
+++ b/plainbox/impl/exporter/jinja2.py
@@ -152,11 +152,18 @@ class Jinja2SessionStateExporter(ISessionStateExporter):
152 Byte stream to write to.152 Byte stream to write to.
153153
154 """154 """
155 try:
156 state = session_manager.state
157 app_blob = state.metadata.app_blob
158 app_blob_data = json.loads(app_blob.decode("UTF-8"))
159 except ValueError:
160 app_blob_data = {}
155 data = {161 data = {
156 'OUTCOME_METADATA_MAP': OUTCOME_METADATA_MAP,162 'OUTCOME_METADATA_MAP': OUTCOME_METADATA_MAP,
157 'client_name': self._client_name,163 'client_name': self._client_name,
158 'client_version': self._client_version,164 'client_version': self._client_version,
159 'manager': session_manager,165 'manager': session_manager,
166 'app_blob': app_blob_data,
160 'options': self.option_list,167 'options': self.option_list,
161 'system_id': self._system_id,168 'system_id': self._system_id,
162 'timestamp': self._timestamp,169 'timestamp': self._timestamp,
diff --git a/plainbox/impl/exporter/test_jinja2.py b/plainbox/impl/exporter/test_jinja2.py
index d35455d..6ae5ce0 100644
--- a/plainbox/impl/exporter/test_jinja2.py
+++ b/plainbox/impl/exporter/test_jinja2.py
@@ -31,6 +31,7 @@ import os
3131
32from plainbox.impl.exporter.jinja2 import Jinja2SessionStateExporter32from plainbox.impl.exporter.jinja2 import Jinja2SessionStateExporter
33from plainbox.impl.result import MemoryJobResult33from plainbox.impl.result import MemoryJobResult
34from plainbox.impl.session.state import SessionMetaData
34from plainbox.impl.unit.exporter import ExporterUnitSupport35from plainbox.impl.unit.exporter import ExporterUnitSupport
35from plainbox.impl.unit.job import JobDefinition36from plainbox.impl.unit.job import JobDefinition
36from plainbox.vendor import mock37from plainbox.vendor import mock
@@ -47,6 +48,7 @@ class Jinja2SessionStateExporterTests(TestCase):
47 job = mock.Mock(spec_set=JobDefinition, id='job_id')48 job = mock.Mock(spec_set=JobDefinition, id='job_id')
48 job.tr_summary.return_value = 'job name'49 job.tr_summary.return_value = 'job name'
49 self.manager_single_job = mock.Mock(state=mock.Mock(50 self.manager_single_job = mock.Mock(state=mock.Mock(
51 metadata=SessionMetaData(),
50 run_list=[job],52 run_list=[job],
51 job_state_map={53 job_state_map={
52 job.id: mock.Mock(result=result, job=job)54 job.id: mock.Mock(result=result, job=job)
diff --git a/plainbox/impl/providers/exporters/data/checkbox.json b/plainbox/impl/providers/exporters/data/checkbox.json
index cd49b05..4257bca 100644
--- a/plainbox/impl/providers/exporters/data/checkbox.json
+++ b/plainbox/impl/providers/exporters/data/checkbox.json
@@ -3,6 +3,11 @@
3{%- set resource_map = state.resource_map -%}3{%- set resource_map = state.resource_map -%}
4{%- set job_state_map = state.job_state_map -%}4{%- set job_state_map = state.job_state_map -%}
5{5{
6{%- if "description" in app_blob %}
7{%- if app_blob['description'] %}
8 "description": {{ app_blob['description'] | jsonify | safe }},
9{%- endif %}
10{%- endif %}
6{%- if ns ~ 'package' in state.resource_map %}11{%- if ns ~ 'package' in state.resource_map %}
7 "packages": [12 "packages": [
8 {%- set package_resource_list = state.resource_map[ns ~ 'package'] %}13 {%- set package_resource_list = state.resource_map[ns ~ 'package'] %}
diff --git a/plainbox/impl/session/state.py b/plainbox/impl/session/state.py
index 825137c..9993689 100644
--- a/plainbox/impl/session/state.py
+++ b/plainbox/impl/session/state.py
@@ -75,7 +75,7 @@ class SessionMetaData:
75 FLAG_TESTPLANLESS = "testplanless"75 FLAG_TESTPLANLESS = "testplanless"
7676
77 def __init__(self, title=None, flags=None, running_job_name=None,77 def __init__(self, title=None, flags=None, running_job_name=None,
78 app_blob=None, app_id=None):78 app_blob=b'', app_id=None):
79 """Initialize a new session state meta-data object."""79 """Initialize a new session state meta-data object."""
80 if flags is None:80 if flags is None:
81 flags = []81 flags = []
diff --git a/plainbox/impl/session/test_state.py b/plainbox/impl/session/test_state.py
index fcb93f7..83fe78f 100644
--- a/plainbox/impl/session/test_state.py
+++ b/plainbox/impl/session/test_state.py
@@ -802,7 +802,7 @@ class SessionMetadataTests(TestCase):
802802
803 def test_app_blob_default_value(self):803 def test_app_blob_default_value(self):
804 metadata = SessionMetaData()804 metadata = SessionMetaData()
805 self.assertIs(metadata.app_blob, None)805 self.assertIs(metadata.app_blob, b'')
806806
807 def test_app_blob_assignment(self):807 def test_app_blob_assignment(self):
808 metadata = SessionMetaData()808 metadata = SessionMetaData()
diff --git a/plainbox/impl/session/test_suspend.py b/plainbox/impl/session/test_suspend.py
index a2f2aad..ab4c6ca 100644
--- a/plainbox/impl/session/test_suspend.py
+++ b/plainbox/impl/session/test_suspend.py
@@ -367,7 +367,7 @@ class SessionSuspendHelper2Tests(SessionSuspendHelper1Tests):
367 'title': None,367 'title': None,
368 'flags': [],368 'flags': [],
369 'running_job_name': None,369 'running_job_name': None,
370 'app_blob': None370 'app_blob': ''
371 })371 })
372372
373 def test_repr_SessionMetaData_typical_metadata(self):373 def test_repr_SessionMetaData_typical_metadata(self):
@@ -403,7 +403,7 @@ class SessionSuspendHelper2Tests(SessionSuspendHelper1Tests):
403 'title': None,403 'title': None,
404 'flags': [],404 'flags': [],
405 'running_job_name': None,405 'running_job_name': None,
406 'app_blob': None,406 'app_blob': '',
407 },407 },
408 })408 })
409409
@@ -427,7 +427,7 @@ class SessionSuspendHelper2Tests(SessionSuspendHelper1Tests):
427 self.assertEqual(gzip.decompress(data), (427 self.assertEqual(gzip.decompress(data), (
428 b'{"session":{"desired_job_list":[],"jobs":{},'428 b'{"session":{"desired_job_list":[],"jobs":{},'
429 b'"mandatory_job_list":[],"metadata":'429 b'"mandatory_job_list":[],"metadata":'
430 b'{"app_blob":null,"flags":[],"running_job_name":null,"title":null'430 b'{"app_blob":"","flags":[],"running_job_name":null,"title":null'
431 b'},"results":{}},"version":2}'))431 b'},"results":{}},"version":2}'))
432432
433433
@@ -458,7 +458,7 @@ class SessionSuspendHelper3Tests(SessionSuspendHelper2Tests):
458 'title': None,458 'title': None,
459 'flags': [],459 'flags': [],
460 'running_job_name': None,460 'running_job_name': None,
461 'app_blob': None,461 'app_blob': '',
462 'app_id': None462 'app_id': None
463 })463 })
464464
@@ -497,7 +497,7 @@ class SessionSuspendHelper3Tests(SessionSuspendHelper2Tests):
497 'title': None,497 'title': None,
498 'flags': [],498 'flags': [],
499 'running_job_name': None,499 'running_job_name': None,
500 'app_blob': None,500 'app_blob': '',
501 'app_id': None,501 'app_id': None,
502 },502 },
503 })503 })
@@ -521,7 +521,7 @@ class SessionSuspendHelper3Tests(SessionSuspendHelper2Tests):
521 self.assertEqual(gzip.decompress(data), (521 self.assertEqual(gzip.decompress(data), (
522 b'{"session":{"desired_job_list":[],"jobs":{},'522 b'{"session":{"desired_job_list":[],"jobs":{},'
523 b'"mandatory_job_list":[],"metadata":'523 b'"mandatory_job_list":[],"metadata":'
524 b'{"app_blob":null,"app_id":null,"flags":[],'524 b'{"app_blob":"","app_id":null,"flags":[],'
525 b'"running_job_name":null,"title":null},"results":{}},'525 b'"running_job_name":null,"title":null},"results":{}},'
526 b'"version":3}'))526 b'"version":3}'))
527527
@@ -590,7 +590,7 @@ class SessionSuspendHelper4Tests(SessionSuspendHelper3Tests):
590 'title': None,590 'title': None,
591 'flags': [],591 'flags': [],
592 'running_job_name': None,592 'running_job_name': None,
593 'app_blob': None,593 'app_blob': '',
594 'app_id': None,594 'app_id': None,
595 },595 },
596 })596 })
@@ -614,7 +614,7 @@ class SessionSuspendHelper4Tests(SessionSuspendHelper3Tests):
614 self.assertEqual(gzip.decompress(data), (614 self.assertEqual(gzip.decompress(data), (
615 b'{"session":{"desired_job_list":[],"jobs":{},'615 b'{"session":{"desired_job_list":[],"jobs":{},'
616 b'"mandatory_job_list":[],"metadata":'616 b'"mandatory_job_list":[],"metadata":'
617 b'{"app_blob":null,"app_id":null,"flags":[],'617 b'{"app_blob":"","app_id":null,"flags":[],'
618 b'"running_job_name":null,"title":null},"results":{}},'618 b'"running_job_name":null,"title":null},"results":{}},'
619 b'"version":4}'))619 b'"version":4}'))
620620
@@ -654,7 +654,7 @@ class SessionSuspendHelper5Tests(SessionSuspendHelper4Tests):
654 self.assertEqual(gzip.decompress(data), (654 self.assertEqual(gzip.decompress(data), (
655 b'{"session":{"desired_job_list":[],"jobs":{},'655 b'{"session":{"desired_job_list":[],"jobs":{},'
656 b'"mandatory_job_list":[],"metadata":'656 b'"mandatory_job_list":[],"metadata":'
657 b'{"app_blob":null,"app_id":null,"flags":[],'657 b'{"app_blob":"","app_id":null,"flags":[],'
658 b'"running_job_name":null,"title":null},"results":{}},'658 b'"running_job_name":null,"title":null},"results":{}},'
659 b'"version":5}'))659 b'"version":5}'))
660660
@@ -694,7 +694,7 @@ class SessionSuspendHelper6Tests(SessionSuspendHelper5Tests):
694 self.assertEqual(gzip.decompress(data), (694 self.assertEqual(gzip.decompress(data), (
695 b'{"session":{"desired_job_list":[],"jobs":{},'695 b'{"session":{"desired_job_list":[],"jobs":{},'
696 b'"mandatory_job_list":[],"metadata":'696 b'"mandatory_job_list":[],"metadata":'
697 b'{"app_blob":null,"app_id":null,"flags":[],'697 b'{"app_blob":"","app_id":null,"flags":[],'
698 b'"running_job_name":null,"title":null},"results":{}},'698 b'"running_job_name":null,"title":null},"results":{}},'
699 b'"version":6}'))699 b'"version":6}'))
700700
@@ -746,7 +746,7 @@ class SessionSuspendHelper6Tests(SessionSuspendHelper5Tests):
746 'title': None,746 'title': None,
747 'flags': [],747 'flags': [],
748 'running_job_name': None,748 'running_job_name': None,
749 'app_blob': None,749 'app_blob': '',
750 'app_id': None,750 'app_id': None,
751 },751 },
752 })752 })
@@ -766,7 +766,7 @@ class SessionSuspendHelper6Tests(SessionSuspendHelper5Tests):
766 'title': None,766 'title': None,
767 'flags': [],767 'flags': [],
768 'running_job_name': None,768 'running_job_name': None,
769 'app_blob': None,769 'app_blob': '',
770 'app_id': None,770 'app_id': None,
771 },771 },
772 })772 })

Subscribers

People subscribed via source and target branches