Merge lp:~allenap/maas/command-line into lp:~maas-committers/maas/trunk
- command-line
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Gavin Panella |
Approved revision: | no longer in the source branch. |
Merged at revision: | 1015 |
Proposed branch: | lp:~allenap/maas/command-line |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
1434 lines (+1227/-53) 17 files modified
buildout.cfg (+2/-0) docs/index.rst (+8/-0) required-packages/base (+1/-0) src/apiclient/maas_client.py (+0/-11) src/apiclient/tests/test_maas_client.py (+0/-14) src/apiclient/tests/test_utils.py (+29/-0) src/apiclient/utils.py (+28/-0) src/maascli/__init__.py (+119/-23) src/maascli/api.py (+336/-0) src/maascli/config.py (+89/-0) src/maascli/tests/test_api.py (+122/-0) src/maascli/tests/test_config.py (+102/-0) src/maascli/tests/test_init.py (+113/-0) src/maascli/tests/test_utils.py (+189/-0) src/maascli/utils.py (+88/-0) src/provisioningserver/tftp.py (+1/-1) versions.cfg (+0/-4) |
To merge this branch: | bzr merge lp:~allenap/maas/command-line |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Raphaël Badin (community) | Approve | ||
Review via email: mp+124704@code.launchpad.net |
Commit message
MAAS command-line client.
Implements support for interacting with MAAS from the command-line. All of MAAS's API is exposed: 'maascli api login ...' connects to a remote server and obtains its API description, automatically augmenting the local client. It's also discoverable: -h/--help can be used to get information on the operations available.
Description of the change
The first iteration of the MAAS command-line API client. It's not fully tested, but hopefully we can all chip in once it's in the tree.
Gavin Panella (allenap) wrote : | # |
Thank you enormously!
> Looks good!
>
> All the registering stuff looks a bit heavy to me but I suppose it's the price
> you pay for making stuff extendable. Same as all the API helpers methods that
> we've created ( ;) ), I don't expect us having to work too much in that area
> once that'll be working as expected though.
>
> This definitely gives us something to work on so I'm approving this even if
> this needs some more work. This obviously won't break anything as it's a
> completely new functionality.
>
> As per our discussion and what I gathered reading that code:
> Things we need to do (I think):
> - Add support for the metadata api
> - Add support for multiple arguments (api/?arg=a&arg=b).
> - Fix encode_
> ASCII string.
> Nice to have:
> - Tab completion (that would be really nice to help humans interacting with
> the API via this CLI).
> - Better separation between anonymous commands and logged-in commands.
>
> Also, the testing coverage seems to be still pretty limited but we definitely
> can iterate on that.
>
> [0]
>
> 398 + if response[
> 399 + raise CommandError(
> 400 + "Expected application/json, got: %(content-type)s" %
> response)
> 401 + return json.loads(content)
>
> I'd also add a try/except block to cope with the possibility of the response
> to be wrongly formatted json.
For now I think that can just bubble up. We can iterate on this.
>
> [1]
>
> 421 + "http://
>
> Maybe change that to "http://
> package uses a '/MAAS' prefix.
Good idea.
>
> [2]
>
> 424 + "The credentials, also known as the API key, for the
> "
> 425 + "remote MAAS server. These can be found in the user "
> 426 + "preferences page in the web UI."
>
> One will be tempted to use his login/password so I'd be a bit more precise
> here to describe what the credentials are (a big string), maybe by giving an
> example of what they look like.
I've added a description, but we can refine this later.
>
> [3]
>
> 687 + "CREATE TABLE IF NOT EXISTS profiles "
> 688 + "(id INTEGER PRIMARY KEY,"
> 689 + " name TEXT NOT NULL UNIQUE,"
> 690 + " data BLOB)")
>
> I wonder why you choose to use that (a sqlite db) over serializing a json
> dictionary into a file? That would have all the nice properties that you have
> here (locking using file locking mechanism, etc.) and it would be more simple
> (to test) and more easy to extend.
I did that first, and I ended up writing code for locking and atomic
writing, and it occurred to me that sqlite provides this all for free.
Fwiw, Zed Shaw came to a similar conclusion when writing the config
system for Mongrel2, and I was inspired by that.
>
> [4]
>
> 250 + if isinstance(module, (str, unicode)):
>
> isinstance(obj, basestring) is a shorter equivalent.
This was my attempt to be forwards compatible, but unicode is not
(yet) an alias for str in Python 3. So...
John A Meinel (jameinel) wrote : | # |
529 + uri = property(lambda self: self.handler[
530 + method = property(lambda self: self.action[
531 + restful = property(lambda self: self.action[
532 + credentials = property(lambda self: self.profile[
533 + op = property(lambda self: self.action["op"])
These look like they might be hard to debug. Would it be better to just put this unwrapping in register_handlers? I wouldn't require it, but it seems if one of them was missing the errors would be hard to understand.
Also, I'd still rather have the tool called 'maas' rather than 'maascli', but I'm willing to be overruled on it. (Having the library and python imports be maascli makes a lot of sense, less-so the actual tool name that gets typed on the commandline.)
Is it possible to minimize/remove the dependency on twisted? It seems like the CLI doesn't actually use it, and it feels like a pretty heavyweight dependency that we aren't using.
I feel like we should split out the code that creates an Action class and get some unittests for it. That can be added later, but it would help to clarify what the structure is supposed to be and what it is supposed to contain.
Gavin Panella (allenap) wrote : | # |
On 19 September 2012 07:45, John A Meinel <email address hidden> wrote:
> 529 + uri = property(lambda self: self.handler[
> 530 + method = property(lambda self: self.action[
> 531 + restful = property(lambda self: self.action[
> 532 + credentials = property(lambda self: self.profile[
> 533 + op = property(lambda self: self.action["op"])
>
> These look like they might be hard to debug. Would it be better to
> just put this unwrapping in register_handlers? I wouldn't require
> it, but it seems if one of them was missing the errors would be hard
> to understand.
I like resolving these values late so that there's no repetition. I
suppose these could be resolved into action_ns in register_actions,
instead of profile, handler, and action, but I think that would
prematurely flatten those structures. I'd rather give the Action
subclass its full context and let it decide what's relevant as late as
possible.
> Also, I'd still rather have the tool called 'maas' rather than
> 'maascli', but I'm willing to be overruled on it. (Having the
> library and python imports be maascli makes a lot of sense, less-so
> the actual tool name that gets typed on the commandline.)
Agreed, I just need to take care of the existing maas script.
> Is it possible to minimize/remove the dependency on twisted? It
> seems like the CLI doesn't actually use it, and it feels like a
> pretty heavyweight dependency that we aren't using.
The dependency is only for testing; the .deb will not depend on
Twisted. We already have Twisted around for other things so pulling it
in for the maascli test suite is almost free.
> I feel like we should split out the code that creates an Action
> class and get some unittests for it. That can be added later, but it
> would help to clarify what the structure is supposed to be and what
> it is supposed to contain.
I agree here too. I wanted to get some tests written for it, but it
became more important to land it for others to see than it was to get
tests in place.
Thanks for reading through it!
Preview Diff
1 | === modified file 'buildout.cfg' |
2 | --- buildout.cfg 2012-09-12 17:01:41 +0000 |
3 | +++ buildout.cfg 2012-09-18 16:56:28 +0000 |
4 | @@ -112,6 +112,7 @@ |
5 | recipe = zc.recipe.egg |
6 | eggs = |
7 | bzr |
8 | + httplib2 |
9 | entry-points = |
10 | maascli=maascli:main |
11 | extra-paths = |
12 | @@ -124,6 +125,7 @@ |
13 | eggs = |
14 | ${maascli:eggs} |
15 | ${common:test-eggs} |
16 | + twisted |
17 | entry-points = |
18 | test.maascli=nose.core:TestProgram |
19 | initialization = |
20 | |
21 | === modified file 'docs/index.rst' |
22 | --- docs/index.rst 2012-09-06 16:32:25 +0000 |
23 | +++ docs/index.rst 2012-09-18 16:56:28 +0000 |
24 | @@ -29,6 +29,14 @@ |
25 | |
26 | Other services interact with a MAAS server using its :doc:`api`. |
27 | |
28 | + |
29 | +MAAS from the command-line |
30 | +========================== |
31 | + |
32 | +MAAS comes with a few command-line tools, including `maascli`, which |
33 | +exposes the entire :doc:`api` to command-line users. |
34 | + |
35 | + |
36 | Code |
37 | ==== |
38 | |
39 | |
40 | === modified file 'required-packages/base' |
41 | --- required-packages/base 2012-09-17 17:27:02 +0000 |
42 | +++ required-packages/base 2012-09-18 16:56:28 +0000 |
43 | @@ -19,6 +19,7 @@ |
44 | python-django |
45 | python-django-south |
46 | python-formencode |
47 | +python-httplib2 |
48 | python-lockfile |
49 | python-netaddr |
50 | python-oauth |
51 | |
52 | === modified file 'src/apiclient/maas_client.py' |
53 | --- src/apiclient/maas_client.py 2012-08-30 06:31:38 +0000 |
54 | +++ src/apiclient/maas_client.py 2012-09-18 16:56:28 +0000 |
55 | @@ -18,22 +18,11 @@ |
56 | |
57 | from urllib import urlencode |
58 | import urllib2 |
59 | -from urlparse import urlparse |
60 | |
61 | from apiclient.multipart import encode_multipart_data |
62 | import oauth.oauth as oauth |
63 | |
64 | |
65 | -def _ascii_url(url): |
66 | - """Encode `url` as ASCII if it isn't already.""" |
67 | - if isinstance(url, unicode): |
68 | - urlparts = urlparse(url) |
69 | - urlparts = urlparts._replace( |
70 | - netloc=urlparts.netloc.encode("idna")) |
71 | - url = urlparts.geturl() |
72 | - return url.encode("ascii") |
73 | - |
74 | - |
75 | class MAASOAuth: |
76 | """Helper class to OAuth-sign an HTTP request.""" |
77 | |
78 | |
79 | === modified file 'src/apiclient/tests/test_maas_client.py' |
80 | --- src/apiclient/tests/test_maas_client.py 2012-08-30 06:31:38 +0000 |
81 | +++ src/apiclient/tests/test_maas_client.py 2012-09-18 16:56:28 +0000 |
82 | @@ -21,7 +21,6 @@ |
83 | ) |
84 | |
85 | from apiclient.maas_client import ( |
86 | - _ascii_url, |
87 | MAASClient, |
88 | MAASDispatcher, |
89 | MAASOAuth, |
90 | @@ -30,19 +29,6 @@ |
91 | from maastesting.testcase import TestCase |
92 | |
93 | |
94 | -class TestHelpers(TestCase): |
95 | - |
96 | - def test_ascii_url_leaves_ascii_bytes_unchanged(self): |
97 | - self.assertEqual( |
98 | - b'http://example.com/', _ascii_url(b'http://example.com/')) |
99 | - self.assertIsInstance(_ascii_url(b'http://example.com'), bytes) |
100 | - |
101 | - def test_ascii_url_asciifies_unicode(self): |
102 | - self.assertEqual( |
103 | - b'http://example.com/', _ascii_url('http://example.com/')) |
104 | - self.assertIsInstance(_ascii_url('http://example.com'), bytes) |
105 | - |
106 | - |
107 | class TestMAASOAuth(TestCase): |
108 | |
109 | def test_sign_request_adds_header(self): |
110 | |
111 | === added file 'src/apiclient/tests/test_utils.py' |
112 | --- src/apiclient/tests/test_utils.py 1970-01-01 00:00:00 +0000 |
113 | +++ src/apiclient/tests/test_utils.py 2012-09-18 16:56:28 +0000 |
114 | @@ -0,0 +1,29 @@ |
115 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
116 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
117 | + |
118 | +"""Test general utilities.""" |
119 | + |
120 | +from __future__ import ( |
121 | + absolute_import, |
122 | + print_function, |
123 | + unicode_literals, |
124 | + ) |
125 | + |
126 | +__metaclass__ = type |
127 | +__all__ = [] |
128 | + |
129 | +from apiclient.utils import ascii_url |
130 | +from maastesting.testcase import TestCase |
131 | + |
132 | + |
133 | +class TestHelpers(TestCase): |
134 | + |
135 | + def test_ascii_url_leaves_ascii_bytes_unchanged(self): |
136 | + self.assertEqual( |
137 | + b'http://example.com/', ascii_url(b'http://example.com/')) |
138 | + self.assertIsInstance(ascii_url(b'http://example.com'), bytes) |
139 | + |
140 | + def test_ascii_url_asciifies_unicode(self): |
141 | + self.assertEqual( |
142 | + b'http://example.com/', ascii_url('http://example.com/')) |
143 | + self.assertIsInstance(ascii_url('http://example.com'), bytes) |
144 | |
145 | === added file 'src/apiclient/utils.py' |
146 | --- src/apiclient/utils.py 1970-01-01 00:00:00 +0000 |
147 | +++ src/apiclient/utils.py 2012-09-18 16:56:28 +0000 |
148 | @@ -0,0 +1,28 @@ |
149 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
150 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
151 | + |
152 | +"""Remote API library.""" |
153 | + |
154 | +from __future__ import ( |
155 | + absolute_import, |
156 | + print_function, |
157 | + unicode_literals, |
158 | + ) |
159 | + |
160 | +__metaclass__ = type |
161 | +__all__ = [ |
162 | + "ascii_url", |
163 | + ] |
164 | + |
165 | + |
166 | +from urlparse import urlparse |
167 | + |
168 | + |
169 | +def ascii_url(url): |
170 | + """Encode `url` as ASCII if it isn't already.""" |
171 | + if isinstance(url, unicode): |
172 | + urlparts = urlparse(url) |
173 | + urlparts = urlparts._replace( |
174 | + netloc=urlparts.netloc.encode("idna")) |
175 | + url = urlparts.geturl() |
176 | + return url.encode("ascii") |
177 | |
178 | === modified file 'src/maascli/__init__.py' |
179 | --- src/maascli/__init__.py 2012-09-12 19:27:01 +0000 |
180 | +++ src/maascli/__init__.py 2012-09-18 16:56:28 +0000 |
181 | @@ -10,30 +10,126 @@ |
182 | ) |
183 | |
184 | __metaclass__ = type |
185 | -__all__ = [] |
186 | +__all__ = [ |
187 | + "Command", |
188 | + "CommandError", |
189 | + "register", |
190 | + ] |
191 | |
192 | -from os.path import ( |
193 | - dirname, |
194 | - join, |
195 | +from abc import ( |
196 | + ABCMeta, |
197 | + abstractmethod, |
198 | ) |
199 | +import argparse |
200 | +import locale |
201 | import sys |
202 | |
203 | -# Add `lib` in this package's directory to sys.path. |
204 | -sys.path.insert(0, join(dirname(__file__), "lib")) |
205 | - |
206 | -from commandant import builtins |
207 | -from commandant.controller import CommandController |
208 | - |
209 | - |
210 | -def main(argv=sys.argv): |
211 | - controller = CommandController( |
212 | - program_name=argv[0], |
213 | - program_version="1.0", |
214 | - program_summary="Control MAAS using its API from the command-line.", |
215 | - program_url="http://maas.ubuntu.com/") |
216 | - # At this point controller.load_path(...) can be used to load commands |
217 | - # from a pre-agreed location on the filesystem, so that the command set |
218 | - # will grow and shrink with the installed packages. |
219 | - controller.load_module(builtins) |
220 | - controller.install_bzrlib_hooks() |
221 | - controller.run(argv[1:]) |
222 | +from bzrlib import osutils |
223 | +from maascli.utils import ( |
224 | + parse_docstring, |
225 | + safe_name, |
226 | + ) |
227 | + |
228 | + |
229 | +modules = { |
230 | + "api": "maascli.api", |
231 | + } |
232 | + |
233 | + |
234 | +class ArgumentParser(argparse.ArgumentParser): |
235 | + """Specialisation of argparse's parser with better support for subparsers. |
236 | + |
237 | + Specifically, the one-shot `add_subparsers` call is disabled, replaced by |
238 | + a lazily evaluated `subparsers` property. |
239 | + """ |
240 | + |
241 | + def add_subparsers(self): |
242 | + raise NotImplementedError( |
243 | + "add_subparsers has been disabled") |
244 | + |
245 | + @property |
246 | + def subparsers(self): |
247 | + try: |
248 | + return self.__subparsers |
249 | + except AttributeError: |
250 | + parent = super(ArgumentParser, self) |
251 | + self.__subparsers = parent.add_subparsers(title="commands") |
252 | + return self.__subparsers |
253 | + |
254 | + |
255 | +def main(argv=None): |
256 | + # Set up the process's locale; this helps bzrlib decode command-line |
257 | + # arguments in the next step. |
258 | + locale.setlocale(locale.LC_ALL, "") |
259 | + if argv is None: |
260 | + argv = sys.argv[:1] + osutils.get_unicode_argv() |
261 | + |
262 | + # Create the base argument parser. |
263 | + parser = ArgumentParser( |
264 | + description="Control MAAS from the command-line.", |
265 | + prog=argv[0], epilog="http://maas.ubuntu.com/") |
266 | + |
267 | + # Register declared modules. |
268 | + for name, module in sorted(modules.items()): |
269 | + if isinstance(module, basestring): |
270 | + module = __import__(module, fromlist=True) |
271 | + help_title, help_body = parse_docstring(module) |
272 | + module_parser = parser.subparsers.add_parser( |
273 | + name, help=help_title, description=help_body) |
274 | + register(module, module_parser) |
275 | + |
276 | + # Run, doing polite things with exceptions. |
277 | + try: |
278 | + options = parser.parse_args(argv[1:]) |
279 | + options.execute(options) |
280 | + except KeyboardInterrupt: |
281 | + raise SystemExit(1) |
282 | + except StandardError as error: |
283 | + parser.error("%s" % error) |
284 | + |
285 | + |
286 | +class Command: |
287 | + """A base class for composing commands. |
288 | + |
289 | + This adheres to the expectations of `register`. |
290 | + """ |
291 | + |
292 | + __metaclass__ = ABCMeta |
293 | + |
294 | + def __init__(self, parser): |
295 | + super(Command, self).__init__() |
296 | + self.parser = parser |
297 | + |
298 | + @abstractmethod |
299 | + def __call__(self, options): |
300 | + """Execute this command.""" |
301 | + |
302 | + |
303 | +CommandError = SystemExit |
304 | + |
305 | + |
306 | +def register(module, parser, prefix="cmd_"): |
307 | + """Register commands in `module` with the given argument parser. |
308 | + |
309 | + This looks for callable objects named `cmd_*` by default, calls them with |
310 | + a new subparser, and registers them as the default value for `execute` in |
311 | + the namespace. |
312 | + |
313 | + If the module also has a `register` function, this is also called, passing |
314 | + in the module being scanned, and the parser given to this function. |
315 | + """ |
316 | + # Register commands. |
317 | + trim = slice(len(prefix), None) |
318 | + commands = { |
319 | + name[trim]: command for name, command in vars(module).items() |
320 | + if name.startswith(prefix) and callable(command) |
321 | + } |
322 | + for name, command in commands.items(): |
323 | + help_title, help_body = parse_docstring(command) |
324 | + command_parser = parser.subparsers.add_parser( |
325 | + safe_name(name), help=help_title, description=help_body) |
326 | + command_parser.set_defaults(execute=command(command_parser)) |
327 | + # Extra subparser registration. |
328 | + register_module = getattr(module, "register", None) |
329 | + if callable(register_module): |
330 | + register_module(module, parser) |
331 | |
332 | === added file 'src/maascli/api.py' |
333 | --- src/maascli/api.py 1970-01-01 00:00:00 +0000 |
334 | +++ src/maascli/api.py 2012-09-18 16:56:28 +0000 |
335 | @@ -0,0 +1,336 @@ |
336 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
337 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
338 | + |
339 | +"""Interact with a remote MAAS server.""" |
340 | + |
341 | +from __future__ import ( |
342 | + absolute_import, |
343 | + print_function, |
344 | + unicode_literals, |
345 | + ) |
346 | + |
347 | +__metaclass__ = type |
348 | +__all__ = [ |
349 | + "register", |
350 | + ] |
351 | + |
352 | +from getpass import getpass |
353 | +import httplib |
354 | +import json |
355 | +import sys |
356 | +from urllib import urlencode |
357 | +from urlparse import ( |
358 | + urljoin, |
359 | + urlparse, |
360 | + ) |
361 | + |
362 | +from apiclient.creds import ( |
363 | + convert_string_to_tuple, |
364 | + convert_tuple_to_string, |
365 | + ) |
366 | +from apiclient.maas_client import MAASOAuth |
367 | +from apiclient.multipart import encode_multipart_data |
368 | +from apiclient.utils import ascii_url |
369 | +import httplib2 |
370 | +from maascli import ( |
371 | + Command, |
372 | + CommandError, |
373 | + ) |
374 | +from maascli.config import ProfileConfig |
375 | +from maascli.utils import ( |
376 | + ensure_trailing_slash, |
377 | + handler_command_name, |
378 | + parse_docstring, |
379 | + safe_name, |
380 | + ) |
381 | + |
382 | + |
383 | +def try_getpass(prompt): |
384 | + """Call `getpass`, ignoring EOF errors.""" |
385 | + try: |
386 | + return getpass(prompt) |
387 | + except EOFError: |
388 | + return None |
389 | + |
390 | + |
391 | +def obtain_credentials(credentials): |
392 | + """Prompt for credentials if possible. |
393 | + |
394 | + If the credentials are "-" then read from stdin without interactive |
395 | + prompting. |
396 | + """ |
397 | + if credentials == "-": |
398 | + credentials = sys.stdin.readline().strip() |
399 | + elif credentials is None: |
400 | + credentials = try_getpass( |
401 | + "API key (leave empty for anonymous access): ") |
402 | + # Ensure that the credentials have a valid form. |
403 | + if credentials and not credentials.isspace(): |
404 | + return convert_string_to_tuple(credentials) |
405 | + else: |
406 | + return None |
407 | + |
408 | + |
409 | +def fetch_api_description(url): |
410 | + """Obtain the description of remote API given its base URL.""" |
411 | + url_describe = urljoin(url, "describe/") |
412 | + http = httplib2.Http() |
413 | + response, content = http.request( |
414 | + ascii_url(url_describe), "GET") |
415 | + if response.status != httplib.OK: |
416 | + raise CommandError( |
417 | + "{0.status} {0.reason}:\n{1}".format(response, content)) |
418 | + if response["content-type"] != "application/json": |
419 | + raise CommandError( |
420 | + "Expected application/json, got: %(content-type)s" % response) |
421 | + return json.loads(content) |
422 | + |
423 | + |
424 | +class cmd_login(Command): |
425 | + """Log-in to a remote API, storing its description and credentials. |
426 | + |
427 | + If credentials are not provided on the command-line, they will be prompted |
428 | + for interactively. |
429 | + """ |
430 | + |
431 | + def __init__(self, parser): |
432 | + super(cmd_login, self).__init__(parser) |
433 | + parser.add_argument( |
434 | + "profile_name", metavar="profile-name", help=( |
435 | + "The name with which you will later refer to this remote " |
436 | + "server and credentials within this tool." |
437 | + )) |
438 | + parser.add_argument( |
439 | + "url", help=( |
440 | + "The URL of the remote API, e.g. " |
441 | + "http://example.com/MAAS/api/1.0/")) |
442 | + parser.add_argument( |
443 | + "credentials", nargs="?", default=None, help=( |
444 | + "The credentials, also known as the API key, for the " |
445 | + "remote MAAS server. These can be found in the user " |
446 | + "preferences page in the web UI; they take the form of " |
447 | + "a long random-looking string composed of three parts, " |
448 | + "separated by colons." |
449 | + )) |
450 | + parser.set_defaults(credentials=None) |
451 | + |
452 | + def __call__(self, options): |
453 | + # Try and obtain credentials interactively if they're not given, or |
454 | + # read them from stdin if they're specified as "-". |
455 | + credentials = obtain_credentials(options.credentials) |
456 | + # Normalise the remote service's URL. |
457 | + url = ensure_trailing_slash(options.url) |
458 | + # Get description of remote API. |
459 | + description = fetch_api_description(url) |
460 | + # Save the config. |
461 | + profile_name = options.profile_name |
462 | + with ProfileConfig.open() as config: |
463 | + config[profile_name] = { |
464 | + "credentials": credentials, |
465 | + "description": description, |
466 | + "name": profile_name, |
467 | + "url": url, |
468 | + } |
469 | + |
470 | + |
471 | +class cmd_refresh(Command): |
472 | + """Refresh the API descriptions of all profiles.""" |
473 | + |
474 | + def __call__(self, options): |
475 | + with ProfileConfig.open() as config: |
476 | + for profile_name in config: |
477 | + profile = config[profile_name] |
478 | + url = profile["url"] |
479 | + profile["description"] = fetch_api_description(url) |
480 | + config[profile_name] = profile |
481 | + |
482 | + |
483 | +class cmd_logout(Command): |
484 | + """Log-out of a remote API, purging any stored credentials.""" |
485 | + |
486 | + def __init__(self, parser): |
487 | + super(cmd_logout, self).__init__(parser) |
488 | + parser.add_argument( |
489 | + "profile_name", metavar="profile-name", help=( |
490 | + "The name with which a remote server and its credentials " |
491 | + "are referred to within this tool." |
492 | + )) |
493 | + |
494 | + def __call__(self, options): |
495 | + with ProfileConfig.open() as config: |
496 | + del config[options.profile_name] |
497 | + |
498 | + |
499 | +class cmd_list(Command): |
500 | + """List remote APIs that have been logged-in to.""" |
501 | + |
502 | + def __call__(self, options): |
503 | + with ProfileConfig.open() as config: |
504 | + for profile_name in config: |
505 | + profile = config[profile_name] |
506 | + url = profile["url"] |
507 | + creds = profile["credentials"] |
508 | + if creds is None: |
509 | + print(profile_name, url) |
510 | + else: |
511 | + creds = convert_tuple_to_string(creds) |
512 | + print(profile_name, url, creds) |
513 | + |
514 | + |
515 | +class Action(Command): |
516 | + """A generic MAAS API action. |
517 | + |
518 | + This is used as a base for creating more specific commands; see |
519 | + `register_actions`. |
520 | + |
521 | + **Note** that this class conflates two things: CLI exposure and API |
522 | + client. The client in apiclient.maas_client is not quite suitable yet, but |
523 | + it should be iterated upon to make it suitable. |
524 | + """ |
525 | + |
526 | + # Override these in subclasses; see `register_actions`. |
527 | + profile = handler = action = None |
528 | + |
529 | + uri = property(lambda self: self.handler["uri"]) |
530 | + method = property(lambda self: self.action["method"]) |
531 | + restful = property(lambda self: self.action["restful"]) |
532 | + credentials = property(lambda self: self.profile["credentials"]) |
533 | + op = property(lambda self: self.action["op"]) |
534 | + |
535 | + def __init__(self, parser): |
536 | + super(Action, self).__init__(parser) |
537 | + for param in self.handler["params"]: |
538 | + parser.add_argument(param) |
539 | + parser.add_argument( |
540 | + "data", type=self.name_value_pair, nargs="*") |
541 | + |
542 | + def __call__(self, options): |
543 | + # TODO: this is el-cheapo URI Template |
544 | + # <http://tools.ietf.org/html/rfc6570> support; use uritemplate-py |
545 | + # <https://github.com/uri-templates/uritemplate-py> here? |
546 | + uri = self.uri.format(**vars(options)) |
547 | + |
548 | + # Parse data out of the positional arguments. |
549 | + data = dict(options.data) |
550 | + if self.op is not None: |
551 | + data["op"] = self.op |
552 | + |
553 | + # Bundle things up ready to throw over the wire. |
554 | + uri, body, headers = self.prepare_payload(uri, data) |
555 | + |
556 | + # Sign request if credentials have been provided. |
557 | + if self.credentials is not None: |
558 | + self.sign(uri, headers, self.credentials) |
559 | + |
560 | + # Use httplib2 instead of urllib2 (or MAASDispatcher, which is based |
561 | + # on urllib2) so that we get full control over HTTP method. TODO: |
562 | + # create custom MAASDispatcher to use httplib2 so that MAASClient can |
563 | + # be used. |
564 | + http = httplib2.Http() |
565 | + response, content = http.request( |
566 | + uri, self.method, body=body, headers=headers) |
567 | + |
568 | + # TODO: decide on how to display responses to users. |
569 | + self.print_response(response, content) |
570 | + |
571 | + # 2xx status codes are all okay. |
572 | + if response.status // 100 != 2: |
573 | + raise CommandError(2) |
574 | + |
575 | + @staticmethod |
576 | + def name_value_pair(string): |
577 | + parts = string.split("=", 1) |
578 | + if len(parts) == 2: |
579 | + return parts |
580 | + else: |
581 | + raise CommandError( |
582 | + "%r is not a name=value pair" % string) |
583 | + |
584 | + def prepare_payload(self, uri, data): |
585 | + """Return the URI (modified perhaps) and body and headers. |
586 | + |
587 | + :param method: The HTTP method. |
588 | + :param uri: The URI of the action. |
589 | + :param data: A dict or iterable of name=value pairs to pack into the |
590 | + body or headers, depending on the type of request. |
591 | + """ |
592 | + if self.method == "POST" and not self.restful: |
593 | + # Encode the data as multipart for non-ReSTful POST requests; all |
594 | + # others should use query parameters. TODO: encode_multipart_data |
595 | + # insists on a dict for the data, which prevents specifying |
596 | + # multiple values for a field, like mac_addresses. This needs to |
597 | + # be fixed. |
598 | + body, headers = encode_multipart_data(data, {}) |
599 | + # TODO: make encode_multipart_data work with arbitrarily encoded |
600 | + # strings; atm, it blows up when encountering a non-ASCII string. |
601 | + else: |
602 | + # TODO: deal with state information, i.e. where to stuff CRUD |
603 | + # data, content types, etc. |
604 | + body, headers = None, {} |
605 | + # TODO: smarter merging of data with query. |
606 | + uri = urlparse(uri)._replace(query=urlencode(data)).geturl() |
607 | + |
608 | + return uri, body, headers |
609 | + |
610 | + @staticmethod |
611 | + def sign(uri, headers, credentials): |
612 | + """Sign the URI and headers.""" |
613 | + auth = MAASOAuth(*credentials) |
614 | + auth.sign_request(uri, headers) |
615 | + |
616 | + @staticmethod |
617 | + def print_response(response, content): |
618 | + """Show an HTTP response in a human-friendly way.""" |
619 | + # Function to change headers like "transfer-encoding" into |
620 | + # "Transfer-Encoding". |
621 | + cap = lambda header: "-".join( |
622 | + part.capitalize() for part in header.split("-")) |
623 | + # Format string to prettify reporting of response headers. |
624 | + form = "%%%ds: %%s" % ( |
625 | + max(len(header) for header in response) + 2) |
626 | + # Print the response. |
627 | + print(response.status, response.reason) |
628 | + print() |
629 | + for header in sorted(response): |
630 | + print(form % (cap(header), response[header])) |
631 | + print() |
632 | + print(content) |
633 | + |
634 | + |
635 | +def register_actions(profile, handler, parser): |
636 | + """Register a handler's actions.""" |
637 | + for action in handler["actions"]: |
638 | + help_title, help_body = parse_docstring(action["doc"]) |
639 | + action_name = safe_name(action["name"]).encode("ascii") |
640 | + action_bases = (Action,) |
641 | + action_ns = { |
642 | + "action": action, |
643 | + "handler": handler, |
644 | + "profile": profile, |
645 | + } |
646 | + action_class = type(action_name, action_bases, action_ns) |
647 | + action_parser = parser.subparsers.add_parser( |
648 | + action_name, help=help_title, description=help_body) |
649 | + action_parser.set_defaults( |
650 | + execute=action_class(action_parser)) |
651 | + |
652 | + |
653 | +def register_handlers(profile, parser): |
654 | + """Register a profile's handlers.""" |
655 | + description = profile["description"] |
656 | + for handler in description["handlers"]: |
657 | + help_title, help_body = parse_docstring(handler["doc"]) |
658 | + handler_name = handler_command_name(handler["name"]) |
659 | + handler_parser = parser.subparsers.add_parser( |
660 | + handler_name, help=help_title, description=help_body) |
661 | + register_actions(profile, handler, handler_parser) |
662 | + |
663 | + |
664 | +def register(module, parser): |
665 | + """Register profiles.""" |
666 | + with ProfileConfig.open() as config: |
667 | + for profile_name in config: |
668 | + profile = config[profile_name] |
669 | + profile_parser = parser.subparsers.add_parser( |
670 | + profile["name"], help="Interact with %(url)s" % profile) |
671 | + register_handlers(profile, profile_parser) |
672 | |
673 | === added file 'src/maascli/config.py' |
674 | --- src/maascli/config.py 1970-01-01 00:00:00 +0000 |
675 | +++ src/maascli/config.py 2012-09-18 16:56:28 +0000 |
676 | @@ -0,0 +1,89 @@ |
677 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
678 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
679 | + |
680 | +"""Configuration abstractions for the MAAS CLI.""" |
681 | + |
682 | +from __future__ import ( |
683 | + absolute_import, |
684 | + print_function, |
685 | + unicode_literals, |
686 | + ) |
687 | + |
688 | +__metaclass__ = type |
689 | +__all__ = [ |
690 | + "ProfileConfig", |
691 | + ] |
692 | + |
693 | +from contextlib import ( |
694 | + closing, |
695 | + contextmanager, |
696 | + ) |
697 | +import json |
698 | +import os |
699 | +from os.path import expanduser |
700 | +import sqlite3 |
701 | + |
702 | + |
703 | +class ProfileConfig: |
704 | + """Store profile configurations in an sqlite3 database.""" |
705 | + |
706 | + def __init__(self, database): |
707 | + self.database = database |
708 | + with self.cursor() as cursor: |
709 | + cursor.execute( |
710 | + "CREATE TABLE IF NOT EXISTS profiles " |
711 | + "(id INTEGER PRIMARY KEY," |
712 | + " name TEXT NOT NULL UNIQUE," |
713 | + " data BLOB)") |
714 | + |
715 | + def cursor(self): |
716 | + return closing(self.database.cursor()) |
717 | + |
718 | + def __iter__(self): |
719 | + with self.cursor() as cursor: |
720 | + results = cursor.execute( |
721 | + "SELECT name FROM profiles").fetchall() |
722 | + return (name for (name,) in results) |
723 | + |
724 | + def __getitem__(self, name): |
725 | + with self.cursor() as cursor: |
726 | + [data] = cursor.execute( |
727 | + "SELECT data FROM profiles" |
728 | + " WHERE name = ?", (name,)).fetchone() |
729 | + return json.loads(data) |
730 | + |
731 | + def __setitem__(self, name, data): |
732 | + with self.cursor() as cursor: |
733 | + cursor.execute( |
734 | + "INSERT OR REPLACE INTO profiles (name, data) " |
735 | + "VALUES (?, ?)", (name, json.dumps(data))) |
736 | + |
737 | + def __delitem__(self, name): |
738 | + with self.cursor() as cursor: |
739 | + cursor.execute( |
740 | + "DELETE FROM profiles" |
741 | + " WHERE name = ?", (name,)) |
742 | + |
743 | + @classmethod |
744 | + @contextmanager |
745 | + def open(cls, dbpath=expanduser("~/.maascli.db")): |
746 | + """Load a profiles database. |
747 | + |
748 | + Called without arguments this will open (and create) a database in the |
749 | + user's home directory. |
750 | + |
751 | + **Note** that this returns a context manager which will close the |
752 | + database on exit, saving if the exit is clean. |
753 | + """ |
754 | + # Initialise filename with restrictive permissions... |
755 | + os.close(os.open(dbpath, os.O_CREAT | os.O_APPEND, 0600)) |
756 | + # before opening it with sqlite. |
757 | + database = sqlite3.connect(dbpath) |
758 | + try: |
759 | + yield cls(database) |
760 | + except: |
761 | + raise |
762 | + else: |
763 | + database.commit() |
764 | + finally: |
765 | + database.close() |
766 | |
767 | === added directory 'src/maascli/tests' |
768 | === added file 'src/maascli/tests/__init__.py' |
769 | === added file 'src/maascli/tests/test_api.py' |
770 | --- src/maascli/tests/test_api.py 1970-01-01 00:00:00 +0000 |
771 | +++ src/maascli/tests/test_api.py 2012-09-18 16:56:28 +0000 |
772 | @@ -0,0 +1,122 @@ |
773 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
774 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
775 | + |
776 | +"""Tests for `maascli.api`.""" |
777 | + |
778 | +from __future__ import ( |
779 | + absolute_import, |
780 | + print_function, |
781 | + unicode_literals, |
782 | + ) |
783 | + |
784 | +__metaclass__ = type |
785 | +__all__ = [] |
786 | + |
787 | +import httplib |
788 | +import json |
789 | +import sys |
790 | + |
791 | +from apiclient.creds import convert_tuple_to_string |
792 | +import httplib2 |
793 | +from maascli import ( |
794 | + api, |
795 | + CommandError, |
796 | + ) |
797 | +from maastesting.factory import factory |
798 | +from maastesting.testcase import TestCase |
799 | +from mock import sentinel |
800 | + |
801 | + |
802 | +class TestFunctions(TestCase): |
803 | + """Test for miscellaneous functions in `maascli.api`.""" |
804 | + |
805 | + def test_try_getpass(self): |
806 | + getpass = self.patch(api, "getpass") |
807 | + getpass.return_value = sentinel.credentials |
808 | + self.assertIs(sentinel.credentials, api.try_getpass(sentinel.prompt)) |
809 | + getpass.assert_called_once_with(sentinel.prompt) |
810 | + |
811 | + def test_try_getpass_eof(self): |
812 | + getpass = self.patch(api, "getpass") |
813 | + getpass.side_effect = EOFError |
814 | + self.assertIsNone(api.try_getpass(sentinel.prompt)) |
815 | + getpass.assert_called_once_with(sentinel.prompt) |
816 | + |
817 | + @staticmethod |
818 | + def make_credentials(): |
819 | + return ( |
820 | + factory.make_name("cred"), |
821 | + factory.make_name("cred"), |
822 | + factory.make_name("cred"), |
823 | + ) |
824 | + |
825 | + def test_obtain_credentials_from_stdin(self): |
826 | + # When "-" is passed to obtain_credentials, it reads credentials from |
827 | + # stdin, trims whitespace, and converts it into a 3-tuple of creds. |
828 | + credentials = self.make_credentials() |
829 | + stdin = self.patch(sys, "stdin") |
830 | + stdin.readline.return_value = ( |
831 | + convert_tuple_to_string(credentials) + "\n") |
832 | + self.assertEqual(credentials, api.obtain_credentials("-")) |
833 | + stdin.readline.assert_called_once() |
834 | + |
835 | + def test_obtain_credentials_via_getpass(self): |
836 | + # When None is passed to obtain_credentials, it attempts to obtain |
837 | + # credentials via getpass, then converts it into a 3-tuple of creds. |
838 | + credentials = self.make_credentials() |
839 | + getpass = self.patch(api, "getpass") |
840 | + getpass.return_value = convert_tuple_to_string(credentials) |
841 | + self.assertEqual(credentials, api.obtain_credentials(None)) |
842 | + getpass.assert_called_once() |
843 | + |
844 | + def test_obtain_credentials_empty(self): |
845 | + # If the entered credentials are empty or only whitespace, |
846 | + # obtain_credentials returns None. |
847 | + getpass = self.patch(api, "getpass") |
848 | + getpass.return_value = None |
849 | + self.assertEqual(None, api.obtain_credentials(None)) |
850 | + getpass.assert_called_once() |
851 | + |
852 | + def test_fetch_api_description(self): |
853 | + content = factory.make_name("content") |
854 | + request = self.patch(httplib2.Http, "request") |
855 | + response = httplib2.Response({}) |
856 | + response.status = httplib.OK |
857 | + response["content-type"] = "application/json" |
858 | + request.return_value = response, json.dumps(content) |
859 | + self.assertEqual( |
860 | + content, api.fetch_api_description("http://example.com/api/1.0/")) |
861 | + request.assert_called_once_with( |
862 | + b"http://example.com/api/1.0/describe/", "GET") |
863 | + |
864 | + def test_fetch_api_description_not_okay(self): |
865 | + # If the response is not 200 OK, fetch_api_description throws toys. |
866 | + content = factory.make_name("content") |
867 | + request = self.patch(httplib2.Http, "request") |
868 | + response = httplib2.Response({}) |
869 | + response.status = httplib.BAD_REQUEST |
870 | + response.reason = httplib.responses[httplib.BAD_REQUEST] |
871 | + request.return_value = response, json.dumps(content) |
872 | + error = self.assertRaises( |
873 | + CommandError, api.fetch_api_description, |
874 | + "http://example.com/api/1.0/") |
875 | + error_expected = "%d %s:\n%s" % ( |
876 | + httplib.BAD_REQUEST, httplib.responses[httplib.BAD_REQUEST], |
877 | + json.dumps(content)) |
878 | + self.assertEqual(error_expected, "%s" % error) |
879 | + |
880 | + def test_fetch_api_description_wrong_content_type(self): |
881 | + # If the response's content type is not application/json, |
882 | + # fetch_api_description throws toys again. |
883 | + content = factory.make_name("content") |
884 | + request = self.patch(httplib2.Http, "request") |
885 | + response = httplib2.Response({}) |
886 | + response.status = httplib.OK |
887 | + response["content-type"] = "text/css" |
888 | + request.return_value = response, json.dumps(content) |
889 | + error = self.assertRaises( |
890 | + CommandError, api.fetch_api_description, |
891 | + "http://example.com/api/1.0/") |
892 | + self.assertEqual( |
893 | + "Expected application/json, got: text/css", |
894 | + "%s" % error) |
895 | |
896 | === added file 'src/maascli/tests/test_config.py' |
897 | --- src/maascli/tests/test_config.py 1970-01-01 00:00:00 +0000 |
898 | +++ src/maascli/tests/test_config.py 2012-09-18 16:56:28 +0000 |
899 | @@ -0,0 +1,102 @@ |
900 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
901 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
902 | + |
903 | +"""Tests for `maascli.config`.""" |
904 | + |
905 | +from __future__ import ( |
906 | + absolute_import, |
907 | + print_function, |
908 | + unicode_literals, |
909 | + ) |
910 | + |
911 | +__metaclass__ = type |
912 | +__all__ = [] |
913 | + |
914 | +import contextlib |
915 | +import os.path |
916 | +import sqlite3 |
917 | + |
918 | +from maascli import api |
919 | +from maastesting.testcase import TestCase |
920 | +from twisted.python.filepath import FilePath |
921 | + |
922 | + |
923 | +class TestProfileConfig(TestCase): |
924 | + """Tests for `ProfileConfig`.""" |
925 | + |
926 | + def test_init(self): |
927 | + database = sqlite3.connect(":memory:") |
928 | + config = api.ProfileConfig(database) |
929 | + with config.cursor() as cursor: |
930 | + # The profiles table has been created. |
931 | + self.assertEqual( |
932 | + cursor.execute( |
933 | + "SELECT COUNT(*) FROM sqlite_master" |
934 | + " WHERE type = 'table'" |
935 | + " AND name = 'profiles'").fetchone(), |
936 | + (1,)) |
937 | + |
938 | + def test_profiles_pristine(self): |
939 | + # A pristine configuration has no profiles. |
940 | + database = sqlite3.connect(":memory:") |
941 | + config = api.ProfileConfig(database) |
942 | + self.assertSetEqual(set(), set(config)) |
943 | + |
944 | + def test_adding_profile(self): |
945 | + database = sqlite3.connect(":memory:") |
946 | + config = api.ProfileConfig(database) |
947 | + config["alice"] = {"abc": 123} |
948 | + self.assertEqual({"alice"}, set(config)) |
949 | + self.assertEqual({"abc": 123}, config["alice"]) |
950 | + |
951 | + def test_replacing_profile(self): |
952 | + database = sqlite3.connect(":memory:") |
953 | + config = api.ProfileConfig(database) |
954 | + config["alice"] = {"abc": 123} |
955 | + config["alice"] = {"def": 456} |
956 | + self.assertEqual({"alice"}, set(config)) |
957 | + self.assertEqual({"def": 456}, config["alice"]) |
958 | + |
959 | + def test_getting_profile(self): |
960 | + database = sqlite3.connect(":memory:") |
961 | + config = api.ProfileConfig(database) |
962 | + config["alice"] = {"abc": 123} |
963 | + self.assertEqual({"abc": 123}, config["alice"]) |
964 | + |
965 | + def test_removing_profile(self): |
966 | + database = sqlite3.connect(":memory:") |
967 | + config = api.ProfileConfig(database) |
968 | + config["alice"] = {"abc": 123} |
969 | + del config["alice"] |
970 | + self.assertEqual(set(), set(config)) |
971 | + |
972 | + def test_open_and_close(self): |
973 | + # ProfileConfig.open() returns a context manager that closes the |
974 | + # database on exit. |
975 | + config_file = os.path.join(self.make_dir(), "config") |
976 | + config = api.ProfileConfig.open(config_file) |
977 | + self.assertIsInstance(config, contextlib.GeneratorContextManager) |
978 | + with config as config: |
979 | + self.assertIsInstance(config, api.ProfileConfig) |
980 | + with config.cursor() as cursor: |
981 | + self.assertEqual( |
982 | + (1,), cursor.execute("SELECT 1").fetchone()) |
983 | + self.assertRaises(sqlite3.ProgrammingError, config.cursor) |
984 | + |
985 | + def test_open_permissions_new_database(self): |
986 | + # ProfileConfig.open() applies restrictive file permissions to newly |
987 | + # created configuration databases. |
988 | + config_file = os.path.join(self.make_dir(), "config") |
989 | + with api.ProfileConfig.open(config_file): |
990 | + perms = FilePath(config_file).getPermissions() |
991 | + self.assertEqual("rw-------", perms.shorthand()) |
992 | + |
993 | + def test_open_permissions_existing_database(self): |
994 | + # ProfileConfig.open() leaves the file permissions of existing |
995 | + # configuration databases. |
996 | + config_file = os.path.join(self.make_dir(), "config") |
997 | + open(config_file, "wb").close() # touch. |
998 | + os.chmod(config_file, 0644) # u=rw,go=r |
999 | + with api.ProfileConfig.open(config_file): |
1000 | + perms = FilePath(config_file).getPermissions() |
1001 | + self.assertEqual("rw-r--r--", perms.shorthand()) |
1002 | |
1003 | === added file 'src/maascli/tests/test_init.py' |
1004 | --- src/maascli/tests/test_init.py 1970-01-01 00:00:00 +0000 |
1005 | +++ src/maascli/tests/test_init.py 2012-09-18 16:56:28 +0000 |
1006 | @@ -0,0 +1,113 @@ |
1007 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
1008 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
1009 | + |
1010 | +"""Tests for `maascli`.""" |
1011 | + |
1012 | +from __future__ import ( |
1013 | + absolute_import, |
1014 | + print_function, |
1015 | + unicode_literals, |
1016 | + ) |
1017 | + |
1018 | +__metaclass__ = type |
1019 | +__all__ = [] |
1020 | + |
1021 | +import new |
1022 | + |
1023 | +from maascli import ( |
1024 | + ArgumentParser, |
1025 | + register, |
1026 | + ) |
1027 | +from maastesting.testcase import TestCase |
1028 | +from mock import sentinel |
1029 | + |
1030 | + |
1031 | +class TestArgumentParser(TestCase): |
1032 | + |
1033 | + def test_add_subparsers_disabled(self): |
1034 | + parser = ArgumentParser() |
1035 | + self.assertRaises(NotImplementedError, parser.add_subparsers) |
1036 | + |
1037 | + def test_subparsers_property(self): |
1038 | + parser = ArgumentParser() |
1039 | + # argparse.ArgumentParser.add_subparsers populates a _subparsers |
1040 | + # attribute when called. Its contents are not the same as the return |
1041 | + # value from add_subparsers, so we just use it an indicator here. |
1042 | + self.assertIsNone(parser._subparsers) |
1043 | + # Reference the subparsers property. |
1044 | + subparsers = parser.subparsers |
1045 | + # _subparsers is populated, meaning add_subparsers has been called on |
1046 | + # the superclass. |
1047 | + self.assertIsNotNone(parser._subparsers) |
1048 | + # The subparsers property, once populated, always returns the same |
1049 | + # object. |
1050 | + self.assertIs(subparsers, parser.subparsers) |
1051 | + |
1052 | + |
1053 | +class TestRegister(TestCase): |
1054 | + """Tests for `maascli.register`.""" |
1055 | + |
1056 | + def test_empty(self): |
1057 | + module = new.module(b"%s.test" % __name__) |
1058 | + parser = ArgumentParser() |
1059 | + register(module, parser) |
1060 | + # No subparsers were registered. |
1061 | + self.assertIsNone(parser._subparsers) |
1062 | + |
1063 | + def test_command(self): |
1064 | + module = new.module(b"%s.test" % __name__) |
1065 | + cmd = self.patch(module, "cmd_one") |
1066 | + cmd.return_value = sentinel.execute |
1067 | + parser = ArgumentParser() |
1068 | + register(module, parser) |
1069 | + # Subparsers were registered. |
1070 | + self.assertIsNotNone(parser._subparsers) |
1071 | + # The command was called once with a subparser called "one". |
1072 | + subparser_one = parser.subparsers.choices["one"] |
1073 | + cmd.assert_called_once_with(subparser_one) |
1074 | + # The subparser has an appropriate execute default. |
1075 | + self.assertIs( |
1076 | + sentinel.execute, |
1077 | + subparser_one.get_default("execute")) |
1078 | + |
1079 | + def test_commands(self): |
1080 | + module = new.module(b"%s.test" % __name__) |
1081 | + cmd_one = self.patch(module, "cmd_one") |
1082 | + cmd_one.return_value = sentinel.x_one |
1083 | + cmd_two = self.patch(module, "cmd_two") |
1084 | + cmd_two.return_value = sentinel.x_two |
1085 | + parser = ArgumentParser() |
1086 | + register(module, parser) |
1087 | + # The commands were called with appropriate subparsers. |
1088 | + subparser_one = parser.subparsers.choices["one"] |
1089 | + cmd_one.assert_called_once_with(subparser_one) |
1090 | + subparser_two = parser.subparsers.choices["two"] |
1091 | + cmd_two.assert_called_once_with(subparser_two) |
1092 | + # The subparsers have appropriate execute defaults. |
1093 | + self.assertIs(sentinel.x_one, subparser_one.get_default("execute")) |
1094 | + self.assertIs(sentinel.x_two, subparser_two.get_default("execute")) |
1095 | + |
1096 | + def test_register(self): |
1097 | + module = new.module(b"%s.test" % __name__) |
1098 | + module_register = self.patch(module, "register") |
1099 | + parser = ArgumentParser() |
1100 | + register(module, parser) |
1101 | + # No subparsers were registered; calling module.register does not |
1102 | + # imply that this happens. |
1103 | + self.assertIsNone(parser._subparsers) |
1104 | + # The command was called once with a subparser called "one". |
1105 | + module_register.assert_called_once_with(module, parser) |
1106 | + |
1107 | + def test_command_and_register(self): |
1108 | + module = new.module(b"%s.test" % __name__) |
1109 | + module_register = self.patch(module, "register") |
1110 | + cmd = self.patch(module, "cmd_one") |
1111 | + parser = ArgumentParser() |
1112 | + register(module, parser) |
1113 | + # Subparsers were registered because a command was found. |
1114 | + self.assertIsNotNone(parser._subparsers) |
1115 | + # The command was called once with a subparser called "one". |
1116 | + module_register.assert_called_once_with(module, parser) |
1117 | + # The command was called once with a subparser called "one". |
1118 | + cmd.assert_called_once_with( |
1119 | + parser.subparsers.choices["one"]) |
1120 | |
1121 | === added file 'src/maascli/tests/test_utils.py' |
1122 | --- src/maascli/tests/test_utils.py 1970-01-01 00:00:00 +0000 |
1123 | +++ src/maascli/tests/test_utils.py 2012-09-18 16:56:28 +0000 |
1124 | @@ -0,0 +1,189 @@ |
1125 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
1126 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
1127 | + |
1128 | +"""Tests for `maascli.utils`.""" |
1129 | + |
1130 | +from __future__ import ( |
1131 | + absolute_import, |
1132 | + print_function, |
1133 | + unicode_literals, |
1134 | + ) |
1135 | + |
1136 | +__metaclass__ = type |
1137 | +__all__ = [] |
1138 | + |
1139 | +from maascli import utils |
1140 | +from maastesting.testcase import TestCase |
1141 | + |
1142 | + |
1143 | +class TestDocstringParsing(TestCase): |
1144 | + """Tests for docstring parsing in `maascli.utils`.""" |
1145 | + |
1146 | + def test_basic(self): |
1147 | + self.assertEqual( |
1148 | + ("Title", "Body"), |
1149 | + utils.parse_docstring("Title\n\nBody")) |
1150 | + self.assertEqual( |
1151 | + ("A longer title", "A longer body"), |
1152 | + utils.parse_docstring( |
1153 | + "A longer title\n\nA longer body")) |
1154 | + |
1155 | + def test_no_body(self): |
1156 | + # parse_docstring returns an empty string when there's no body. |
1157 | + self.assertEqual( |
1158 | + ("Title", ""), |
1159 | + utils.parse_docstring("Title\n\n")) |
1160 | + self.assertEqual( |
1161 | + ("Title", ""), |
1162 | + utils.parse_docstring("Title")) |
1163 | + |
1164 | + def test_unwrapping(self): |
1165 | + # parse_docstring dedents and unwraps the title and body paragraphs. |
1166 | + self.assertEqual( |
1167 | + ("Title over two lines", |
1168 | + "Paragraph over two lines\n\n" |
1169 | + "Another paragraph over two lines"), |
1170 | + utils.parse_docstring(""" |
1171 | + Title over |
1172 | + two lines |
1173 | + |
1174 | + Paragraph over |
1175 | + two lines |
1176 | + |
1177 | + Another paragraph |
1178 | + over two lines |
1179 | + """)) |
1180 | + |
1181 | + def test_no_unwrapping_for_indented_paragraphs(self): |
1182 | + # parse_docstring dedents body paragraphs, but does not unwrap those |
1183 | + # with indentation beyond the rest. |
1184 | + self.assertEqual( |
1185 | + ("Title over two lines", |
1186 | + "Paragraph over two lines\n\n" |
1187 | + " An indented paragraph\n which will remain wrapped\n\n" |
1188 | + "Another paragraph over two lines"), |
1189 | + utils.parse_docstring(""" |
1190 | + Title over |
1191 | + two lines |
1192 | + |
1193 | + Paragraph over |
1194 | + two lines |
1195 | + |
1196 | + An indented paragraph |
1197 | + which will remain wrapped |
1198 | + |
1199 | + Another paragraph |
1200 | + over two lines |
1201 | + """)) |
1202 | + |
1203 | + def test_gets_docstring_from_function(self): |
1204 | + # parse_docstring can extract the docstring when the argument passed |
1205 | + # is not a string type. |
1206 | + def example(): |
1207 | + """Title. |
1208 | + |
1209 | + Body. |
1210 | + """ |
1211 | + self.assertEqual( |
1212 | + ("Title.", "Body."), |
1213 | + utils.parse_docstring(example)) |
1214 | + |
1215 | + def test_normalises_whitespace(self): |
1216 | + # parse_docstring can parse CRLF/CR/LF text, but always emits LF (\n, |
1217 | + # new-line) separated text. |
1218 | + self.assertEqual( |
1219 | + ("long title", ""), |
1220 | + utils.parse_docstring("long\r\ntitle")) |
1221 | + self.assertEqual( |
1222 | + ("title", "body1\n\nbody2"), |
1223 | + utils.parse_docstring("title\n\nbody1\r\rbody2")) |
1224 | + |
1225 | + |
1226 | +class TestFunctions(TestCase): |
1227 | + """Tests for miscellaneous functions in `maascli.utils`.""" |
1228 | + |
1229 | + maxDiff = TestCase.maxDiff * 2 |
1230 | + |
1231 | + def test_safe_name(self): |
1232 | + # safe_name attempts to discriminate parts of a vaguely camel-cased |
1233 | + # string, and rejoins them using a hyphen. |
1234 | + expected = { |
1235 | + "NodeHandler": "Node-Handler", |
1236 | + "SpadeDiggingHandler": "Spade-Digging-Handler", |
1237 | + "SPADE_Digging_Handler": "SPADE-Digging-Handler", |
1238 | + "SpadeHandlerForDigging": "Spade-Handler-For-Digging", |
1239 | + "JamesBond007": "James-Bond007", |
1240 | + "JamesBOND": "James-BOND", |
1241 | + "James-BOND-007": "James-BOND-007", |
1242 | + } |
1243 | + observed = { |
1244 | + name_in: utils.safe_name(name_in) |
1245 | + for name_in in expected |
1246 | + } |
1247 | + self.assertItemsEqual( |
1248 | + expected.items(), observed.items()) |
1249 | + |
1250 | + def test_safe_name_non_ASCII(self): |
1251 | + # safe_name will not break if passed a string with non-ASCII |
1252 | + # characters. However, those characters will not be present in the |
1253 | + # returned name. |
1254 | + self.assertEqual( |
1255 | + "a-b-c", utils.safe_name(u"a\u1234_b\u5432_c\u9876")) |
1256 | + |
1257 | + def test_safe_name_string_type(self): |
1258 | + # Given a unicode string, safe_name will always return a unicode |
1259 | + # string, and given a byte string it will always return a byte string. |
1260 | + self.assertIsInstance(utils.safe_name(u"fred"), unicode) |
1261 | + self.assertIsInstance(utils.safe_name(b"fred"), bytes) |
1262 | + |
1263 | + def test_handler_command_name(self): |
1264 | + # handler_command_name attempts to discriminate parts of a vaguely |
1265 | + # camel-cased string, removes any "handler" parts, joins again with |
1266 | + # hyphens, and returns the whole lot in lower case. |
1267 | + expected = { |
1268 | + "NodeHandler": "node", |
1269 | + "SpadeDiggingHandler": "spade-digging", |
1270 | + "SPADE_Digging_Handler": "spade-digging", |
1271 | + "SpadeHandlerForDigging": "spade-for-digging", |
1272 | + "JamesBond007": "james-bond007", |
1273 | + "JamesBOND": "james-bond", |
1274 | + "James-BOND-007": "james-bond-007", |
1275 | + } |
1276 | + observed = { |
1277 | + name_in: utils.handler_command_name(name_in) |
1278 | + for name_in in expected |
1279 | + } |
1280 | + self.assertItemsEqual( |
1281 | + expected.items(), observed.items()) |
1282 | + # handler_command_name also ensures that all names are encoded into |
1283 | + # byte strings. |
1284 | + expected_types = { |
1285 | + name_out: bytes |
1286 | + for name_out in observed.values() |
1287 | + } |
1288 | + observed_types = { |
1289 | + name_out: type(name_out) |
1290 | + for name_out in observed.values() |
1291 | + } |
1292 | + self.assertItemsEqual( |
1293 | + expected_types.items(), observed_types.items()) |
1294 | + |
1295 | + def test_handler_command_name_non_ASCII(self): |
1296 | + # handler_command_name will not break if passed a string with |
1297 | + # non-ASCII characters. However, those characters will not be present |
1298 | + # in the returned name. |
1299 | + self.assertEqual( |
1300 | + "a-b-c", utils.handler_command_name(u"a\u1234_b\u5432_c\u9876")) |
1301 | + |
1302 | + def test_ensure_trailing_slash(self): |
1303 | + # ensure_trailing_slash ensures that the given string - typically a |
1304 | + # URL or path - has a trailing forward slash. |
1305 | + self.assertEqual("fred/", utils.ensure_trailing_slash("fred")) |
1306 | + self.assertEqual("fred/", utils.ensure_trailing_slash("fred/")) |
1307 | + |
1308 | + def test_ensure_trailing_slash_string_type(self): |
1309 | + # Given a unicode string, ensure_trailing_slash will always return a |
1310 | + # unicode string, and given a byte string it will always return a byte |
1311 | + # string. |
1312 | + self.assertIsInstance(utils.ensure_trailing_slash(u"fred"), unicode) |
1313 | + self.assertIsInstance(utils.ensure_trailing_slash(b"fred"), bytes) |
1314 | |
1315 | === added file 'src/maascli/utils.py' |
1316 | --- src/maascli/utils.py 1970-01-01 00:00:00 +0000 |
1317 | +++ src/maascli/utils.py 2012-09-18 16:56:28 +0000 |
1318 | @@ -0,0 +1,88 @@ |
1319 | +# Copyright 2012 Canonical Ltd. This software is licensed under the |
1320 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
1321 | + |
1322 | +"""Utilities for the command-line interface.""" |
1323 | + |
1324 | +from __future__ import ( |
1325 | + absolute_import, |
1326 | + print_function, |
1327 | + unicode_literals, |
1328 | + ) |
1329 | + |
1330 | +__metaclass__ = type |
1331 | +__all__ = [ |
1332 | + "ensure_trailing_slash", |
1333 | + "handler_command_name", |
1334 | + "parse_docstring", |
1335 | + "safe_name", |
1336 | + ] |
1337 | + |
1338 | +from functools import partial |
1339 | +from inspect import getdoc |
1340 | +import re |
1341 | +from textwrap import dedent |
1342 | + |
1343 | + |
1344 | +re_paragraph_splitter = re.compile( |
1345 | + r"(?:\r\n){2,}|\r{2,}|\n{2,}", re.MULTILINE) |
1346 | + |
1347 | +paragraph_split = re_paragraph_splitter.split |
1348 | +docstring_split = partial(paragraph_split, maxsplit=1) |
1349 | +remove_line_breaks = lambda string: ( |
1350 | + " ".join(line.strip() for line in string.splitlines())) |
1351 | + |
1352 | +newline = "\n" |
1353 | +empty = "" |
1354 | + |
1355 | + |
1356 | +def parse_docstring(thing): |
1357 | + doc = thing if isinstance(thing, (str, unicode)) else getdoc(thing) |
1358 | + doc = empty if doc is None else doc.expandtabs().strip() |
1359 | + # Break the docstring into two parts: title and body. |
1360 | + parts = docstring_split(doc) |
1361 | + if len(parts) == 2: |
1362 | + title, body = parts[0], dedent(parts[1]) |
1363 | + else: |
1364 | + title, body = parts[0], empty |
1365 | + # Remove line breaks from the title line. |
1366 | + title = remove_line_breaks(title) |
1367 | + # Remove line breaks from non-indented paragraphs in the body. |
1368 | + paragraphs = [] |
1369 | + for paragraph in paragraph_split(body): |
1370 | + if not paragraph[:1].isspace(): |
1371 | + paragraph = remove_line_breaks(paragraph) |
1372 | + paragraphs.append(paragraph) |
1373 | + # Rejoin the paragraphs, normalising on newline. |
1374 | + body = (newline + newline).join( |
1375 | + paragraph.replace("\r\n", newline).replace("\r", newline) |
1376 | + for paragraph in paragraphs) |
1377 | + return title, body |
1378 | + |
1379 | + |
1380 | +re_camelcase = re.compile( |
1381 | + r"([A-Z]*[a-z0-9]+|[A-Z]+)(?:(?=[^a-z0-9])|\Z)") |
1382 | + |
1383 | + |
1384 | +def safe_name(string): |
1385 | + """Return a munged version of string, suitable as an ASCII filename.""" |
1386 | + hyphen = "-" if isinstance(string, unicode) else b"-" |
1387 | + return hyphen.join(re_camelcase.findall(string)) |
1388 | + |
1389 | + |
1390 | +def handler_command_name(string): |
1391 | + """Create a handler command name from an arbitrary string. |
1392 | + |
1393 | + Camel-case parts of string will be extracted, converted to lowercase, |
1394 | + joined with hyphens, and the rest discarded. The term "handler" will also |
1395 | + be removed if discovered amongst the aforementioned parts. |
1396 | + """ |
1397 | + parts = re_camelcase.findall(string) |
1398 | + parts = (part.lower().encode("ascii") for part in parts) |
1399 | + parts = (part for part in parts if part != b"handler") |
1400 | + return b"-".join(parts) |
1401 | + |
1402 | + |
1403 | +def ensure_trailing_slash(string): |
1404 | + """Ensure that `string` has a trailing forward-slash.""" |
1405 | + slash = b"/" if isinstance(string, bytes) else u"/" |
1406 | + return (string + slash) if not string.endswith(slash) else string |
1407 | |
1408 | === modified file 'src/provisioningserver/tftp.py' |
1409 | --- src/provisioningserver/tftp.py 2012-08-30 10:47:03 +0000 |
1410 | +++ src/provisioningserver/tftp.py 2012-09-18 16:56:28 +0000 |
1411 | @@ -124,7 +124,7 @@ |
1412 | # Merge updated query into the generator URL. |
1413 | url = self.generator_url._replace(query=urlencode(query)) |
1414 | # TODO: do something more intelligent with unicode URLs here; see |
1415 | - # maas_client._ascii_url() for inspiration. |
1416 | + # apiclient.utils.ascii_url() for inspiration. |
1417 | return url.geturl().encode("ascii") |
1418 | |
1419 | @deferred |
1420 | |
1421 | === modified file 'versions.cfg' |
1422 | --- versions.cfg 2012-09-12 17:01:41 +0000 |
1423 | +++ versions.cfg 2012-09-18 16:56:28 +0000 |
1424 | @@ -96,10 +96,6 @@ |
1425 | |
1426 | # Required by: |
1427 | # entrypoint2==0.0.4 |
1428 | -argparse = 1.2.1 |
1429 | - |
1430 | -# Required by: |
1431 | -# entrypoint2==0.0.4 |
1432 | decorator = 3.3.2 |
1433 | |
1434 | # Required by: |
Looks good!
All the registering stuff looks a bit heavy to me but I suppose it's the price you pay for making stuff extendable. Same as all the API helpers methods that we've created ( ;) ), I don't expect us having to work too much in that area once that'll be working as expected though.
This definitely gives us something to work on so I'm approving this even if this needs some more work. This obviously won't break anything as it's a completely new functionality.
As per our discussion and what I gathered reading that code: multipart_ data so that it won't blow up when encountering a non-ASCII string.
Things we need to do (I think):
- Add support for the metadata api
- Add support for multiple arguments (api/?arg=a&arg=b).
- Fix encode_
Nice to have:
- Tab completion (that would be really nice to help humans interacting with the API via this CLI).
- Better separation between anonymous commands and logged-in commands.
Also, the testing coverage seems to be still pretty limited but we definitely can iterate on that.
[0]
398 + if response[ "content- type"] != "application/json":
399 + raise CommandError(
400 + "Expected application/json, got: %(content-type)s" % response)
401 + return json.loads(content)
I'd also add a try/except block to cope with the possibility of the response to be wrongly formatted json.
[1]
421 + "http:// example. com/api/ 1.0/"))
Maybe change that to "http:// example. com/MAAS/ api/1.0/" since the default package uses a '/MAAS' prefix.
[2]
424 + "The credentials, also known as the API key, for the "
425 + "remote MAAS server. These can be found in the user "
426 + "preferences page in the web UI."
One will be tempted to use his login/password so I'd be a bit more precise here to describe what the credentials are (a big string), maybe by giving an example of what they look like.
[3]
687 + "CREATE TABLE IF NOT EXISTS profiles "
688 + "(id INTEGER PRIMARY KEY,"
689 + " name TEXT NOT NULL UNIQUE,"
690 + " data BLOB)")
I wonder why you choose to use that (a sqlite db) over serializing a json dictionary into a file? That would have all the nice properties that you have here (locking using file locking mechanism, etc.) and it would be more simple (to test) and more easy to extend.
[4]
250 + if isinstance(module, (str, unicode)):
isinstance(obj, basestring) is a shorter equivalent.
[5]
$ ./bin/maascli api admin node read node-33b55e28- 4671-11e1- 93b8-00225f89f2 11
200 OK
Content- Location: http:// 192.168. 0.3:5240/ api/1.0/ nodes/node- 33b55e28- 4671-11e1- 93b8-00225f89f2 11/
Content- Type: application/json; charset=utf-8
Date: Tue, 18 Sep 2012 11:10:34 GMT
Server: WSGIServer/0.1 Python/2.7.3
Status: 200 Encoding: chunked
Vary: Authorization, Cookie
Transfer-
[...]
I don't think this should be displayed unless in a verbose mode. I'm talking about the "200 OK" bit and the headers.
595 + """Show an HTTP response in a human-friendly way."""
I suspect that CLI will be used by humans but also a lot by sc...