Merge ~sylvain-pineau/checkbox-ng:fix-session-type into checkbox-ng:master

Proposed by Sylvain Pineau
Status: Merged
Approved by: Sylvain Pineau
Approved revision: fa3949881890c7fccab10919d189826770f437a2
Merged at revision: cb6088e126c805dde0f4ebb326a7ef826875e9d7
Proposed branch: ~sylvain-pineau/checkbox-ng:fix-session-type
Merge into: checkbox-ng:master
Diff against target: 32 lines (+9/-5)
1 file modified
plainbox/impl/session/assistant.py (+9/-5)
Reviewer Review Type Date Requested Status
Maciej Kisielewski Approve
Checkbox Developers Pending
Review via email: mp+417976@code.launchpad.net

Description of the change

checkbox remote is broken (again) since:

https://git.launchpad.net/checkbox-ng/commit/?id=511e6f6e7f2e29ed994dabfba614097d722caa08

The way the session type is extracted from session titles does not work as launchers are setting
a different value causing the type to fallback to "local".

The patch evaluates the session type based on the session metadata app blob instead.

Tested using the dev ppa version (to verify the behavior of the deb restart strategy) and this patch applied manually (on jammy beta)

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

Neverending story :)
LGTM, +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/plainbox/impl/session/assistant.py b/plainbox/impl/session/assistant.py
2index 7afe1a9..41ec411 100644
3--- a/plainbox/impl/session/assistant.py
4+++ b/plainbox/impl/session/assistant.py
5@@ -247,7 +247,7 @@ class SessionAssistant:
6 if self._restart_strategy is None:
7 # 'checkbox-slave' is deprecated, it's here so people can resume
8 # old session, the next if statement can be changed to just checking
9- # for 'remote' title
10+ # for 'remote' type
11 # session_type = 'remote' if self._metadata.title == 'remote'
12 # else 'local'
13 # with the next release or when we do inclusive naming refactor
14@@ -255,10 +255,14 @@ class SessionAssistant:
15 # TODO: REMOTE API RAPI:
16 # this heuristic of guessing session type from the title
17 # should be changed to a proper arg/flag with the Remote API bump
18- remote_titles = ('remote', 'checkbox-slave')
19- if self._metadata and self._metadata.title in remote_titles:
20- session_type = 'remote'
21- else:
22+ remote_types = ('remote', 'checkbox-slave')
23+ session_type = 'local'
24+ try:
25+ app_blob = json.loads(self._metadata.app_blob.decode("UTF-8"))
26+ session_type = app_blob['type']
27+ if session_type in remote_types:
28+ session_type = 'remote'
29+ except (AttributeError, ValueError, KeyError):
30 session_type = 'local'
31 self._restart_strategy = detect_restart_strategy(
32 self, session_type=session_type)

Subscribers

People subscribed via source and target branches