Merge lp:~sylvain-pineau/checkbox/fix-1578579 into lp:checkbox

Proposed by Sylvain Pineau
Status: Merged
Approved by: Sylvain Pineau
Approved revision: 4349
Merged at revision: 4352
Proposed branch: lp:~sylvain-pineau/checkbox/fix-1578579
Merge into: lp:checkbox
Diff against target: 31 lines (+2/-6)
2 files modified
checkbox-ng/launchers/checkbox-cli (+0/-6)
plainbox/plainbox/impl/session/assistant.py (+2/-0)
To merge this branch: bzr merge lp:~sylvain-pineau/checkbox/fix-1578579
Reviewer Review Type Date Requested Status
Pierre Equoy Approve
Review via email: mp+294711@code.launchpad.net

Description of the change

Fixes the linked bug.

To avoid the plainbox crash mentioned in the bug report where we had leftover jobs when resuming a session a simple fix is to avoid rerunning jobs in the bootstrap method if we already have results for them. Doing so avoid generating a new set of jobs from templates, where one could be missing if udev does not report the same device list (It can be as simple as unplugging a USB mouse).

To post a comment you must log in.
Revision history for this message
Pierre Equoy (pieq) wrote :

I tested this patch on the device where lp:1578579 was found, and it does, indeed, fix the problem.

I thought it might introduce a regression with the job results history, but after testing it a bit more it seems OK and the whole history is kept even when the session is resumed.

+1, thanks a lot!

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

The job history is not always kept, I just avoid rerun the bootstrap include jobs on resume which is often limited to 3-4 resource jobs (device, package, graphic cards and fwts)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'checkbox-ng/launchers/checkbox-cli'
--- checkbox-ng/launchers/checkbox-cli 2016-05-10 18:04:53 +0000
+++ checkbox-ng/launchers/checkbox-cli 2016-05-14 14:07:58 +0000
@@ -312,14 +312,8 @@
312 app_blob = json.loads(metadata.app_blob.decode("UTF-8"))312 app_blob = json.loads(metadata.app_blob.decode("UTF-8"))
313 test_plan_id = app_blob['testplan_id']313 test_plan_id = app_blob['testplan_id']
314 last_job = metadata.running_job_name314 last_job = metadata.running_job_name
315 # FIXME selecting again the testplan on resume resets both # the static
316 # and dynamic todo lists. # We're then saving the selection from the
317 # saved run_list # by accessing the session private context object.
318 selected_id_list = [job.id for job in
319 self.ctx.sa._context.state.run_list]
320 self.ctx.sa.select_test_plan(test_plan_id)315 self.ctx.sa.select_test_plan(test_plan_id)
321 self.ctx.sa.bootstrap()316 self.ctx.sa.bootstrap()
322 self.ctx.sa.use_alternate_selection(selected_id_list)
323 # If we resumed maybe not rerun the same, probably broken job317 # If we resumed maybe not rerun the same, probably broken job
324 self._handle_last_job_after_resume(last_job)318 self._handle_last_job_after_resume(last_job)
325319
326320
=== modified file 'plainbox/plainbox/impl/session/assistant.py'
--- plainbox/plainbox/impl/session/assistant.py 2016-04-04 18:24:10 +0000
+++ plainbox/plainbox/impl/session/assistant.py 2016-05-14 14:07:58 +0000
@@ -852,6 +852,8 @@
852 self._context.state.update_desired_job_list(852 self._context.state.update_desired_job_list(
853 desired_job_list, include_mandatory=False)853 desired_job_list, include_mandatory=False)
854 for job in self._context.state.run_list:854 for job in self._context.state.run_list:
855 if self._context.state.job_state_map[job.id].result_history:
856 continue
855 UsageExpectation.of(self).allowed_calls[self.run_job] = (857 UsageExpectation.of(self).allowed_calls[self.run_job] = (
856 "to run bootstrapping job")858 "to run bootstrapping job")
857 rb = self.run_job(job.id, 'silent', False)859 rb = self.run_job(job.id, 'silent', False)

Subscribers

People subscribed via source and target branches