Merge lp:~mew/charm-helpers/document-core into lp:charm-helpers
- document-core
- Merge into devel
Proposed by
Matthew Wedgwood
Status: | Merged |
---|---|
Merged at revision: | 85 |
Proposed branch: | lp:~mew/charm-helpers/document-core |
Merge into: | lp:charm-helpers |
Diff against target: |
396 lines (+68/-32) 2 files modified
charmhelpers/core/hookenv.py (+53/-23) charmhelpers/core/host.py (+15/-9) |
To merge this branch: | bzr merge lp:~mew/charm-helpers/document-core |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Page | Approve | ||
Review via email: mp+187623@code.launchpad.net |
Commit message
Description of the change
Add or clean up docstrings for all core functions.
To post a comment you must log in.
Revision history for this message
Nicola Larosa (teknico) wrote : | # |
- 80. By Matthew Wedgwood
-
Standardize docstring quoting in core
Revision history for this message
Matthew Wedgwood (mew) wrote : | # |
I've fixed the docstring quoting in the core modules.
Revision history for this message
James Page (james-page) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'charmhelpers/core/hookenv.py' |
2 | --- charmhelpers/core/hookenv.py 2013-07-18 16:13:49 +0000 |
3 | +++ charmhelpers/core/hookenv.py 2013-09-26 14:36:53 +0000 |
4 | @@ -21,7 +21,7 @@ |
5 | |
6 | |
7 | def cached(func): |
8 | - ''' Cache return values for multiple executions of func + args |
9 | + """Cache return values for multiple executions of func + args |
10 | |
11 | For example: |
12 | |
13 | @@ -32,7 +32,7 @@ |
14 | unit_get('test') |
15 | |
16 | will cache the result of unit_get + 'test' for future calls. |
17 | - ''' |
18 | + """ |
19 | def wrapper(*args, **kwargs): |
20 | global cache |
21 | key = str((func, args, kwargs)) |
22 | @@ -46,8 +46,8 @@ |
23 | |
24 | |
25 | def flush(key): |
26 | - ''' Flushes any entries from function cache where the |
27 | - key is found in the function+args ''' |
28 | + """Flushes any entries from function cache where the |
29 | + key is found in the function+args """ |
30 | flush_list = [] |
31 | for item in cache: |
32 | if key in item: |
33 | @@ -57,7 +57,7 @@ |
34 | |
35 | |
36 | def log(message, level=None): |
37 | - "Write a message to the juju log" |
38 | + """Write a message to the juju log""" |
39 | command = ['juju-log'] |
40 | if level: |
41 | command += ['-l', level] |
42 | @@ -66,7 +66,7 @@ |
43 | |
44 | |
45 | class Serializable(UserDict.IterableUserDict): |
46 | - "Wrapper, an object that can be serialized to yaml or json" |
47 | + """Wrapper, an object that can be serialized to yaml or json""" |
48 | |
49 | def __init__(self, obj): |
50 | # wrap the object |
51 | @@ -96,11 +96,11 @@ |
52 | self.data = state |
53 | |
54 | def json(self): |
55 | - "Serialize the object to json" |
56 | + """Serialize the object to json""" |
57 | return json.dumps(self.data) |
58 | |
59 | def yaml(self): |
60 | - "Serialize the object to yaml" |
61 | + """Serialize the object to yaml""" |
62 | return yaml.dump(self.data) |
63 | |
64 | |
65 | @@ -119,38 +119,38 @@ |
66 | |
67 | |
68 | def in_relation_hook(): |
69 | - "Determine whether we're running in a relation hook" |
70 | + """Determine whether we're running in a relation hook""" |
71 | return 'JUJU_RELATION' in os.environ |
72 | |
73 | |
74 | def relation_type(): |
75 | - "The scope for the current relation hook" |
76 | + """The scope for the current relation hook""" |
77 | return os.environ.get('JUJU_RELATION', None) |
78 | |
79 | |
80 | def relation_id(): |
81 | - "The relation ID for the current relation hook" |
82 | + """The relation ID for the current relation hook""" |
83 | return os.environ.get('JUJU_RELATION_ID', None) |
84 | |
85 | |
86 | def local_unit(): |
87 | - "Local unit ID" |
88 | + """Local unit ID""" |
89 | return os.environ['JUJU_UNIT_NAME'] |
90 | |
91 | |
92 | def remote_unit(): |
93 | - "The remote unit for the current relation hook" |
94 | + """The remote unit for the current relation hook""" |
95 | return os.environ['JUJU_REMOTE_UNIT'] |
96 | |
97 | |
98 | def service_name(): |
99 | - "The name service group this unit belongs to" |
100 | + """The name service group this unit belongs to""" |
101 | return local_unit().split('/')[0] |
102 | |
103 | |
104 | @cached |
105 | def config(scope=None): |
106 | - "Juju charm configuration" |
107 | + """Juju charm configuration""" |
108 | config_cmd_line = ['config-get'] |
109 | if scope is not None: |
110 | config_cmd_line.append(scope) |
111 | @@ -163,6 +163,7 @@ |
112 | |
113 | @cached |
114 | def relation_get(attribute=None, unit=None, rid=None): |
115 | + """Get relation information""" |
116 | _args = ['relation-get', '--format=json'] |
117 | if rid: |
118 | _args.append('-r') |
119 | @@ -177,6 +178,7 @@ |
120 | |
121 | |
122 | def relation_set(relation_id=None, relation_settings={}, **kwargs): |
123 | + """Set relation information for the current unit""" |
124 | relation_cmd_line = ['relation-set'] |
125 | if relation_id is not None: |
126 | relation_cmd_line.extend(('-r', relation_id)) |
127 | @@ -192,7 +194,7 @@ |
128 | |
129 | @cached |
130 | def relation_ids(reltype=None): |
131 | - "A list of relation_ids" |
132 | + """A list of relation_ids""" |
133 | reltype = reltype or relation_type() |
134 | relid_cmd_line = ['relation-ids', '--format=json'] |
135 | if reltype is not None: |
136 | @@ -203,7 +205,7 @@ |
137 | |
138 | @cached |
139 | def related_units(relid=None): |
140 | - "A list of related units" |
141 | + """A list of related units""" |
142 | relid = relid or relation_id() |
143 | units_cmd_line = ['relation-list', '--format=json'] |
144 | if relid is not None: |
145 | @@ -213,7 +215,7 @@ |
146 | |
147 | @cached |
148 | def relation_for_unit(unit=None, rid=None): |
149 | - "Get the json represenation of a unit's relation" |
150 | + """Get the json represenation of a unit's relation""" |
151 | unit = unit or remote_unit() |
152 | relation = relation_get(unit=unit, rid=rid) |
153 | for key in relation: |
154 | @@ -225,7 +227,7 @@ |
155 | |
156 | @cached |
157 | def relations_for_id(relid=None): |
158 | - "Get relations of a specific relation ID" |
159 | + """Get relations of a specific relation ID""" |
160 | relation_data = [] |
161 | relid = relid or relation_ids() |
162 | for unit in related_units(relid): |
163 | @@ -237,7 +239,7 @@ |
164 | |
165 | @cached |
166 | def relations_of_type(reltype=None): |
167 | - "Get relations of a specific type" |
168 | + """Get relations of a specific type""" |
169 | relation_data = [] |
170 | reltype = reltype or relation_type() |
171 | for relid in relation_ids(reltype): |
172 | @@ -249,7 +251,7 @@ |
173 | |
174 | @cached |
175 | def relation_types(): |
176 | - "Get a list of relation types supported by this charm" |
177 | + """Get a list of relation types supported by this charm""" |
178 | charmdir = os.environ.get('CHARM_DIR', '') |
179 | mdf = open(os.path.join(charmdir, 'metadata.yaml')) |
180 | md = yaml.safe_load(mdf) |
181 | @@ -264,6 +266,7 @@ |
182 | |
183 | @cached |
184 | def relations(): |
185 | + """Get a nested dictionary of relation data for all related units""" |
186 | rels = {} |
187 | for reltype in relation_types(): |
188 | relids = {} |
189 | @@ -278,14 +281,14 @@ |
190 | |
191 | |
192 | def open_port(port, protocol="TCP"): |
193 | - "Open a service network port" |
194 | + """Open a service network port""" |
195 | _args = ['open-port'] |
196 | _args.append('{}/{}'.format(port, protocol)) |
197 | subprocess.check_call(_args) |
198 | |
199 | |
200 | def close_port(port, protocol="TCP"): |
201 | - "Close a service network port" |
202 | + """Close a service network port""" |
203 | _args = ['close-port'] |
204 | _args.append('{}/{}'.format(port, protocol)) |
205 | subprocess.check_call(_args) |
206 | @@ -293,6 +296,7 @@ |
207 | |
208 | @cached |
209 | def unit_get(attribute): |
210 | + """Get the unit ID for the remote unit""" |
211 | _args = ['unit-get', '--format=json', attribute] |
212 | try: |
213 | return json.loads(subprocess.check_output(_args)) |
214 | @@ -301,22 +305,46 @@ |
215 | |
216 | |
217 | def unit_private_ip(): |
218 | + """Get this unit's private IP address""" |
219 | return unit_get('private-address') |
220 | |
221 | |
222 | class UnregisteredHookError(Exception): |
223 | + """Raised when an undefined hook is called""" |
224 | pass |
225 | |
226 | |
227 | class Hooks(object): |
228 | + """A convenient handler for hook functions. |
229 | + |
230 | + Example: |
231 | + hooks = Hooks() |
232 | + |
233 | + # register a hook, taking its name from the function name |
234 | + @hooks.hook() |
235 | + def install(): |
236 | + ... |
237 | + |
238 | + # register a hook, providing a custom hook name |
239 | + @hooks.hook("config-changed") |
240 | + def config_changed(): |
241 | + ... |
242 | + |
243 | + if __name__ == "__main__": |
244 | + # execute a hook based on the name the program is called by |
245 | + hooks.execute(sys.argv) |
246 | + """ |
247 | + |
248 | def __init__(self): |
249 | super(Hooks, self).__init__() |
250 | self._hooks = {} |
251 | |
252 | def register(self, name, function): |
253 | + """Register a hook""" |
254 | self._hooks[name] = function |
255 | |
256 | def execute(self, args): |
257 | + """Execute a registered hook based on args[0]""" |
258 | hook_name = os.path.basename(args[0]) |
259 | if hook_name in self._hooks: |
260 | self._hooks[hook_name]() |
261 | @@ -324,6 +352,7 @@ |
262 | raise UnregisteredHookError(hook_name) |
263 | |
264 | def hook(self, *hook_names): |
265 | + """Decorator, registering them as hooks""" |
266 | def wrapper(decorated): |
267 | for hook_name in hook_names: |
268 | self.register(hook_name, decorated) |
269 | @@ -337,4 +366,5 @@ |
270 | |
271 | |
272 | def charm_dir(): |
273 | + """Return the root directory of the current charm""" |
274 | return os.environ.get('CHARM_DIR') |
275 | |
276 | === modified file 'charmhelpers/core/host.py' |
277 | --- charmhelpers/core/host.py 2013-08-23 16:42:43 +0000 |
278 | +++ charmhelpers/core/host.py 2013-09-26 14:36:53 +0000 |
279 | @@ -19,18 +19,22 @@ |
280 | |
281 | |
282 | def service_start(service_name): |
283 | + """Start a system service""" |
284 | return service('start', service_name) |
285 | |
286 | |
287 | def service_stop(service_name): |
288 | + """Stop a system service""" |
289 | return service('stop', service_name) |
290 | |
291 | |
292 | def service_restart(service_name): |
293 | + """Restart a system service""" |
294 | return service('restart', service_name) |
295 | |
296 | |
297 | def service_reload(service_name, restart_on_failure=False): |
298 | + """Reload a system service, optionally falling back to restart if reload fails""" |
299 | service_result = service('reload', service_name) |
300 | if not service_result and restart_on_failure: |
301 | service_result = service('restart', service_name) |
302 | @@ -38,11 +42,13 @@ |
303 | |
304 | |
305 | def service(action, service_name): |
306 | + """Control a system service""" |
307 | cmd = ['service', service_name, action] |
308 | return subprocess.call(cmd) == 0 |
309 | |
310 | |
311 | def service_running(service): |
312 | + """Determine whether a system service is running""" |
313 | try: |
314 | output = subprocess.check_output(['service', service, 'status']) |
315 | except subprocess.CalledProcessError: |
316 | @@ -55,7 +61,7 @@ |
317 | |
318 | |
319 | def adduser(username, password=None, shell='/bin/bash', system_user=False): |
320 | - """Add a user""" |
321 | + """Add a user to the system""" |
322 | try: |
323 | user_info = pwd.getpwnam(username) |
324 | log('user {0} already exists!'.format(username)) |
325 | @@ -138,7 +144,7 @@ |
326 | |
327 | |
328 | def mount(device, mountpoint, options=None, persist=False): |
329 | - '''Mount a filesystem''' |
330 | + """Mount a filesystem at a particular mountpoint""" |
331 | cmd_args = ['mount'] |
332 | if options is not None: |
333 | cmd_args.extend(['-o', options]) |
334 | @@ -155,7 +161,7 @@ |
335 | |
336 | |
337 | def umount(mountpoint, persist=False): |
338 | - '''Unmount a filesystem''' |
339 | + """Unmount a filesystem""" |
340 | cmd_args = ['umount', mountpoint] |
341 | try: |
342 | subprocess.check_output(cmd_args) |
343 | @@ -169,7 +175,7 @@ |
344 | |
345 | |
346 | def mounts(): |
347 | - '''List of all mounted volumes as [[mountpoint,device],[...]]''' |
348 | + """Get a list of all mounted volumes as [[mountpoint,device],[...]]""" |
349 | with open('/proc/mounts') as f: |
350 | # [['/mount/point','/dev/path'],[...]] |
351 | system_mounts = [m[1::-1] for m in [l.strip().split() |
352 | @@ -178,7 +184,7 @@ |
353 | |
354 | |
355 | def file_hash(path): |
356 | - ''' Generate a md5 hash of the contents of 'path' or None if not found ''' |
357 | + """Generate a md5 hash of the contents of 'path' or None if not found """ |
358 | if os.path.exists(path): |
359 | h = hashlib.md5() |
360 | with open(path, 'r') as source: |
361 | @@ -189,7 +195,7 @@ |
362 | |
363 | |
364 | def restart_on_change(restart_map): |
365 | - ''' Restart services based on configuration files changing |
366 | + """Restart services based on configuration files changing |
367 | |
368 | This function is used a decorator, for example |
369 | |
370 | @@ -202,7 +208,7 @@ |
371 | In this example, the cinder-api and cinder-volume services |
372 | would be restarted if /etc/ceph/ceph.conf is changed by the |
373 | ceph_client_changed function. |
374 | - ''' |
375 | + """ |
376 | def wrap(f): |
377 | def wrapped_f(*args): |
378 | checksums = {} |
379 | @@ -220,7 +226,7 @@ |
380 | |
381 | |
382 | def lsb_release(): |
383 | - '''Return /etc/lsb-release in a dict''' |
384 | + """Return /etc/lsb-release in a dict""" |
385 | d = {} |
386 | with open('/etc/lsb-release', 'r') as lsb: |
387 | for l in lsb: |
388 | @@ -230,7 +236,7 @@ |
389 | |
390 | |
391 | def pwgen(length=None): |
392 | - '''Generate a random pasword.''' |
393 | + """Generate a random pasword.""" |
394 | if length is None: |
395 | length = random.choice(range(35, 45)) |
396 | alphanumeric_chars = [ |
Very much appreciated, thank you.
However, please be consistent in your choice of docstring delimiters. The convention is to always use triple double-quotes (""").
There's actually a whole guideline for docstrings alone <http:// www.python. org/dev/ peps/pep- 0257/>.
Inconsistency in this has no use that I know of.