Merge ~ballot/apache2-charm:lp1863860 into apache2-charm:master

Proposed by Benjamin Allot
Status: Merged
Approved by: Benjamin Allot
Approved revision: 03e4413152a7a66449741eca872d4266f408d860
Merged at revision: 316a6124af02daf8f5ae7df29f38136495975682
Proposed branch: ~ballot/apache2-charm:lp1863860
Merge into: apache2-charm:master
Diff against target: 85 lines (+13/-7)
3 files modified
.gitignore (+2/-0)
hooks/hooks.py (+6/-5)
hooks/tests/test_balancer_hook.py (+5/-2)
Reviewer Review Type Date Requested Status
Junien F Approve
Canonical IS Reviewers Pending
Review via email: mp+379454@code.launchpad.net

Commit message

Fix re.split usage and add the same logic for purge
Also fix the tests.

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Junien F (axino) wrote :

+1

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 316a6124af02daf8f5ae7df29f38136495975682

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/.gitignore b/.gitignore
2new file mode 100644
3index 0000000..042ece2
4--- /dev/null
5+++ b/.gitignore
6@@ -0,0 +1,2 @@
7+/.venv
8+*.pyc
9diff --git a/hooks/hooks.py b/hooks/hooks.py
10index 0c652f7..08f7622 100755
11--- a/hooks/hooks.py
12+++ b/hooks/hooks.py
13@@ -52,12 +52,12 @@ juju_warning_header = """#
14 # Supporting functions
15 ###############################################################################
16
17-def apt_get_install(package=None):
18- """Install a package."""
19- if package is None:
20+def apt_get_install(packages=None):
21+ """Install packages."""
22+ if packages is None:
23 return False
24 cmd_line = ['apt-get', '-y', 'install', '-qq']
25- for pkg in re.split(' |,'):
26+ for pkg in re.split(' |,', packages):
27 cmd_line.append(pkg)
28 return subprocess.call(cmd_line)
29
30@@ -78,7 +78,8 @@ def apt_get_purge(packages=None):
31 if packages is None:
32 return False
33 cmd_line = ['apt-get', '-y', 'purge', '-qq']
34- cmd_line.append(packages)
35+ for pkg in re.split(' |,', packages):
36+ cmd_line.append(pkg)
37 return subprocess.call(cmd_line)
38
39
40diff --git a/hooks/tests/test_balancer_hook.py b/hooks/tests/test_balancer_hook.py
41index 6b5aacb..d808ec8 100644
42--- a/hooks/tests/test_balancer_hook.py
43+++ b/hooks/tests/test_balancer_hook.py
44@@ -135,7 +135,7 @@ class HelpersTest(TestCase):
45
46 self.assertEqual(result, 'some result')
47 mock_call.assert_called_with(['apt-get', '-y', 'install', '-qq',
48- 'foo bar'])
49+ 'foo', 'bar'])
50
51 @patch('subprocess.call')
52 def test_installs_nothing_if_package_not_provided(self, mock_call):
53@@ -150,7 +150,7 @@ class HelpersTest(TestCase):
54
55 self.assertEqual(result, 'some result')
56 mock_call.assert_called_with(['apt-get', '-y', 'purge', '-qq',
57- 'foo bar'])
58+ 'foo', 'bar'])
59
60 @patch('subprocess.call')
61 def test_purges_nothing_if_package_not_provided(self, mock_call):
62@@ -519,6 +519,7 @@ class HooksTest(TestCase):
63 0o600)
64 apt_get_install.assert_has_calls([
65 call('python-jinja2'),
66+ call('python-openssl'),
67 call('python-pyasn1'),
68 call('python-pyasn1-modules'),
69 call('python-yaml'),
70@@ -545,6 +546,7 @@ class HooksTest(TestCase):
71 self.assertEqual(result, 'some result')
72 apt_get_install.assert_has_calls([
73 call('python-jinja2'),
74+ call('python-openssl'),
75 call('python-pyasn1'),
76 call('python-pyasn1-modules'),
77 call('python-yaml'),
78@@ -573,6 +575,7 @@ class HooksTest(TestCase):
79 self.assertFalse(mkdir.called)
80 apt_get_install.assert_has_calls([
81 call('python-jinja2'),
82+ call('python-openssl'),
83 call('python-pyasn1'),
84 call('python-pyasn1-modules'),
85 call('python-yaml'),

Subscribers

People subscribed via source and target branches