Sudo_pair Charm

Merge ~peppepetra/charm-sudo-pair:lint-20.08 into charm-sudo-pair:master

  1. Git
  2. lp:~peppepetra/charm-sudo-pair
  3. lint-20.08
  4. Merge into master
Proposed by Giuseppe Petralia
Status: Merged
Merged at revision: 7e3bae46ebed59798e7bfd6d4fe66a14c208d08e
Proposed branch: ~peppepetra/charm-sudo-pair:lint-20.08
Merge into: charm-sudo-pair:master
Prerequisite: ~peppepetra/charm-sudo-pair:blacken-20.08
Diff against target: 276 lines (+49/-29)
9 files modified
src/actions/actions.py (+2/-0)
src/lib/libsudopair.py (+1/-0)
src/reactive/sudo_pair.py (+4/-0)
src/tests/functional/conftest.py (+27/-26)
src/tests/functional/test_deploy.py (+1/-0)
src/tests/unit/conftest.py (+4/-0)
src/tests/unit/test_actions.py (+3/-0)
src/tests/unit/test_libsudopair.py (+4/-0)
src/tox.ini (+3/-3)
Reviewer Review Type Date Requested Status
Drew Freiberger (community) Approve
Paul Goins Approve
Review via email: mp+388999@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Paul Goins (vultaire) wrote :

Generally in favor, but the noqa clauses need to be fixed since without the colon they disable all checks.

review: Needs Fixing
Revision history for this message
Paul Goins (vultaire) wrote :

LGTM.

review: Approve
Revision history for this message
Drew Freiberger (afreiberger) wrote :

LGTM. +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/actions/actions.py b/src/actions/actions.py
2index 2416eb3..9ff1ec5 100755
3--- a/src/actions/actions.py
4+++ b/src/actions/actions.py
5@@ -1,4 +1,5 @@
6 #!/usr/local/sbin/charm-env python3
7+"""Sudo Pair actions."""
8 #
9 # Copyright 2016,2019,2020 Canonical Ltd
10 #
11@@ -39,6 +40,7 @@ ACTIONS = {"remove-sudopair": remove}
12
13
14 def main(args):
15+ """Dispatch actions based on command arguments."""
16 action_name = os.path.basename(args[0])
17 try:
18 ACTIONS[action_name]()
19diff --git a/src/lib/libsudopair.py b/src/lib/libsudopair.py
20index cbaf467..2f7c45b 100644
21--- a/src/lib/libsudopair.py
22+++ b/src/lib/libsudopair.py
23@@ -1,3 +1,4 @@
24+"""Sudo pair utilities."""
25 import grp
26 import os
27
28diff --git a/src/reactive/sudo_pair.py b/src/reactive/sudo_pair.py
29index ed1d7ba..caab64f 100644
30--- a/src/reactive/sudo_pair.py
31+++ b/src/reactive/sudo_pair.py
32@@ -1,3 +1,5 @@
33+"""Charm reactive hooks."""
34+
35 from charmhelpers.core import hookenv
36
37 from charms.reactive import hook, remove_state, set_state, when, when_not
38@@ -46,11 +48,13 @@ def install_sudo_pair():
39
40 @hook("config-changed")
41 def reconfigure_sudo_pair_charm():
42+ """Run config-changed hook."""
43 sph.set_charm_config(hookenv.config())
44 remove_state("sudo-pair.configured")
45
46
47 @hook("stop")
48 def stop():
49+ """Run stop hook and remove charm configuration."""
50 sph.deconfigure()
51 remove_state("sudo-pair.installed")
52diff --git a/src/tests/functional/conftest.py b/src/tests/functional/conftest.py
53index 7a8523e..fdd4356 100644
54--- a/src/tests/functional/conftest.py
55+++ b/src/tests/functional/conftest.py
56@@ -1,4 +1,5 @@
57 #!/usr/bin/python3
58+"""Functional tests utilities."""
59
60 import asyncio
61 import json
62@@ -60,7 +61,7 @@ async def current_model():
63
64
65 @pytest.fixture
66-async def get_app(model):
67+async def get_app(model): # noqa: D202
68 """Return the application requested."""
69
70 async def _get_app(name):
71@@ -73,7 +74,7 @@ async def get_app(model):
72
73
74 @pytest.fixture
75-async def get_unit(model):
76+async def get_unit(model): # noqa: D202
77 """Return the requested <app_name>/<unit_number> unit."""
78
79 async def _get_unit(name):
80@@ -87,7 +88,7 @@ async def get_unit(model):
81
82
83 @pytest.fixture
84-async def get_entity(model, get_unit, get_app):
85+async def get_entity(model, get_unit, get_app): # noqa: D202
86 """Return a unit or an application."""
87
88 async def _get_entity(name):
89@@ -103,15 +104,15 @@ async def get_entity(model, get_unit, get_app):
90
91
92 @pytest.fixture
93-async def run_command(get_unit):
94- """
95- Run a command on a unit.
96-
97- :param cmd: Command to be run
98- :param target: Unit object or unit name string
99- """
100+async def run_command(get_unit): # noqa: D202
101+ """Run a command on an unit."""
102
103 async def _run_command(cmd, target):
104+ """Run a command on a unit.
105+
106+ :param cmd: Command to be run
107+ :param target: Unit object or unit name string
108+ """
109 unit = target if type(target) is juju.unit.Unit else await get_unit(target)
110 action = await unit.run(cmd)
111 return action.results
112@@ -120,15 +121,15 @@ async def run_command(get_unit):
113
114
115 @pytest.fixture
116-async def file_stat(run_command):
117- """
118- Run stat on a file.
119-
120- :param path: File path
121- :param target: Unit object or unit name string
122- """
123+async def file_stat(run_command): # noqa: D202
124+ """Get file stat from an unit."""
125
126 async def _file_stat(path, target):
127+ """Run stat on a file.
128+
129+ :param path: File path
130+ :param target: Unit object or unit name string
131+ """
132 cmd = STAT_FILE % path
133 results = await run_command(cmd, target)
134 return json.loads(results["Stdout"])
135@@ -137,15 +138,15 @@ async def file_stat(run_command):
136
137
138 @pytest.fixture
139-async def file_contents(run_command):
140- """
141- Return the contents of a file.
142-
143- :param path: File path
144- :param target: Unit object or unit name string
145- """
146+async def file_contents(run_command): # noqa: D202
147+ """Return the contents of a file."""
148
149 async def _file_contents(path, target):
150+ """Return the contents of a file.
151+
152+ :param path: File path
153+ :param target: Unit object or unit name string
154+ """
155 cmd = "cat {}".format(path)
156 results = await run_command(cmd, target)
157 return results["Stdout"]
158@@ -154,7 +155,7 @@ async def file_contents(run_command):
159
160
161 @pytest.fixture
162-async def reconfigure_app(get_app, model):
163+async def reconfigure_app(get_app, model): # noqa: D202
164 """Apply a different config to the requested app."""
165
166 async def _reconfigure_app(cfg, target):
167@@ -171,7 +172,7 @@ async def reconfigure_app(get_app, model):
168
169
170 @pytest.fixture
171-async def create_group(run_command):
172+async def create_group(run_command): # noqa: D202
173 """Create the UNIX group specified."""
174
175 async def _create_group(group_name, target):
176diff --git a/src/tests/functional/test_deploy.py b/src/tests/functional/test_deploy.py
177index 7f24224..11e8297 100644
178--- a/src/tests/functional/test_deploy.py
179+++ b/src/tests/functional/test_deploy.py
180@@ -1,4 +1,5 @@
181 #!/usr/bin/python3.6
182+"""Charm functional tests."""
183
184 import os
185
186diff --git a/src/tests/unit/conftest.py b/src/tests/unit/conftest.py
187index 9ca50ec..c1df1d0 100644
188--- a/src/tests/unit/conftest.py
189+++ b/src/tests/unit/conftest.py
190@@ -1,4 +1,5 @@
191 #!/usr/bin/python3
192+"""Unit tests utilities."""
193
194 import grp
195 import os
196@@ -10,6 +11,7 @@ import pytest
197
198 @pytest.fixture
199 def mock_hookenv_config(monkeypatch):
200+ """Mock hookenv config."""
201 import yaml
202
203 def mock_config():
204@@ -27,11 +29,13 @@ def mock_hookenv_config(monkeypatch):
205
206 @pytest.fixture
207 def mock_charm_dir(monkeypatch):
208+ """Mock charm dir."""
209 monkeypatch.setattr("charmhelpers.core.hookenv.charm_dir", lambda: ".")
210
211
212 @pytest.fixture
213 def sph(mock_hookenv_config, mock_charm_dir, tmpdir):
214+ """Return SudoPairHelpers with mocks."""
215 from libsudopair import SudoPairHelper
216
217 sph = SudoPairHelper()
218diff --git a/src/tests/unit/test_actions.py b/src/tests/unit/test_actions.py
219index d6d86d7..ddf6003 100644
220--- a/src/tests/unit/test_actions.py
221+++ b/src/tests/unit/test_actions.py
222@@ -1,3 +1,5 @@
223+"""Unit tests for charm actions."""
224+
225 import os
226 import sys
227 import unittest.mock as mock
228@@ -11,6 +13,7 @@ import actions # NOQA
229 @mock.patch("actions.action_set")
230 @mock.patch("actions.status_set")
231 def test_remove_action(status_set, action_set, sudo_pair_helper):
232+ """Verify remove action."""
233 actions.remove()
234 msg = "Successfully removed sudo-pair config and binaries"
235 action_set.assert_called_with({"message": msg})
236diff --git a/src/tests/unit/test_libsudopair.py b/src/tests/unit/test_libsudopair.py
237index 874f0a3..860d8db 100644
238--- a/src/tests/unit/test_libsudopair.py
239+++ b/src/tests/unit/test_libsudopair.py
240@@ -1,3 +1,5 @@
241+"""Sudopair lib unit tests."""
242+
243 import filecmp
244 import grp
245 import os
246@@ -6,11 +8,13 @@ from libsudopair import check_valid_group, group_id
247
248
249 def test_check_valid_group():
250+ """Check an unix group is valid."""
251 assert not check_valid_group("fake_group")
252 assert check_valid_group(grp.getgrgid(os.getgid()).gr_name)
253
254
255 def test_group_id():
256+ """Verify group_id() is correct."""
257 assert group_id(grp.getgrgid(os.getgid()).gr_name) == os.getgid()
258
259
260diff --git a/src/tox.ini b/src/tox.ini
261index 2085ea9..2900ead 100644
262--- a/src/tox.ini
263+++ b/src/tox.ini
264@@ -29,9 +29,9 @@ commands =
265 deps =
266 black
267 flake8
268-#TODO flake8-docstrings
269-#TODO flake8-import-order
270-#TODO pep8-naming
271+ flake8-docstrings
272+ flake8-import-order
273+ pep8-naming
274 flake8-colors
275
276 [flake8]

Subscribers

People subscribed via source and target branches