Merge lp:~brendan-donegan/checkbox/bug1013537_nmcli_con_up_return_code into lp:checkbox

Proposed by Brendan Donegan
Status: Merged
Merged at revision: 1439
Proposed branch: lp:~brendan-donegan/checkbox/bug1013537_nmcli_con_up_return_code
Merge into: lp:checkbox
Diff against target: 25 lines (+3/-1)
2 files modified
debian/changelog (+2/-0)
scripts/create_connection (+1/-1)
To merge this branch: bzr merge lp:~brendan-donegan/checkbox/bug1013537_nmcli_con_up_return_code
Reviewer Review Type Date Requested Status
Marc Tardif (community) Approve
Brendan Donegan (community) Needs Resubmitting
Review via email: mp+110486@code.launchpad.net

Description of the change

Originally, after calling nmcli con up the script was looking at its output to check for a string to confirm the connection was activated. In Quantal it has no output, so we need to check the return code. Note that I have not used subprocess.call, because that ends up spitting out all of the messages when run in Precise. Instead I do communicate and discard stdout, and then check the returncode property of Popen

To post a comment you must log in.
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Also, I could have used 'if !nmcli_con_up.returncode', but I hate to use negation in a situation where I'm checking for success.

1440. By Brendan Donegan

Put the changes described in the changelog.

Revision history for this message
Marc Tardif (cr3) wrote :

Have you tested these changes on Precise too?

review: Needs Information
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Yes! In fact the better question would be 'have I tested them on Quantal' :)

review: Needs Resubmitting
Revision history for this message
Marc Tardif (cr3) wrote :

It seemed that you had already tested Quantal: In Quantal it has no output, so we need to check the return code.

While merging, I renamed stderr to dummy because Popen was not called with stderr=PIPE. So, stderr always contained None which was misleading. No biggie.

review: Approve
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

No problem. In Precise it *does* have an output, but still returns a code all the same - so it works in both just as well

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2012-06-13 20:09:23 +0000
3+++ debian/changelog 2012-06-15 08:28:19 +0000
4@@ -11,6 +11,8 @@
5 encoding issues
6 * Update touchpad.py to use gsettings instead of deprecated gconf
7 (LP: #1004212)
8+ * Instead of checking output of nmcli con up in create_connection,
9+ check the return code is success instead (LP: #1013537)
10
11 [Marc Tardif]
12 * [FEATURE] Reworked media_keys_test into key_test, making it more generic
13
14=== modified file 'scripts/create_connection'
15--- scripts/create_connection 2012-06-07 14:17:22 +0000
16+++ scripts/create_connection 2012-06-15 08:28:19 +0000
17@@ -88,7 +88,7 @@
18 stdout=PIPE)
19 (stdout, stderr) = nmcli_con_up.communicate()
20
21- if 'state: activated' in stdout.decode():
22+ if nmcli_con_up.returncode == 0:
23 print("Connection %s activated." % connection)
24 else:
25 print("Failed to activate %s." % connection)

Subscribers

People subscribed via source and target branches