Merge lp:~javier.collado/utah/bug1025220 into lp:utah

[Javier Collado]

Initially I moved the URL checking code to the loop in utah/provisioning/provisioning.py to make sure that all arguments that expect a URL are properly checked.

However, after that, I thought that it would make more sense to use the `type` argument in argparse.ArgumentParser.add_argument to mark each of those arguments as expecting a URL and perform the validation right away.

The validation code still lies in utah.url, but now it's called by argparse when parsing the arguments.

[Max Brustkern (nuclearbob)]

Good idea. We can easily leverage that in future classes as well.

review: Approve

=== modified file 'examples/run_utah_tests.py'
--- examples/run_utah_tests.py	2012-07-16 14:45:15 +0000
+++ examples/run_utah_tests.py	2012-07-17 14:31:55 +0000
@@ -4,22 +4,22 @@
 import sys
 
 from utah.exceptions import URLNotFound, URLNotReadable
-from utah.url import URLChecker
+from utah.url import url_argument
 from utah.group import check_user_group, print_group_error_message
 from run_test_vm import run_test_vm
 
 
 def get_parser():
     parser = argparse.ArgumentParser(description='Provision a machine and run one or more UTAH runlists there.', epilog="For example:\n\t%(prog)s -s oneiric -t server -a i386 /usr/share/utah/client/examples/master.run 'http://people.canonical.com/~max/max_test.run'", formatter_class=argparse.RawDescriptionHelpFormatter)
-    parser.add_argument('runlists', metavar='runlist', nargs='+', help='URLs of runlist files to run')
+    parser.add_argument('runlists', metavar='runlist', nargs='+', type=url_argument, help='URLs of runlist files to run')
     parser.add_argument('-m', '--machinetype', choices=('physical', 'virtual'), default='virtual', help='Type of machine to provision')
     parser.add_argument('-e', '--emulator', help='Emulator to use (kvm and qemu are supported, kvm will be favored if available)')
-    parser.add_argument('-i', '--image', help='Image/ISO file to use for installation')
-    parser.add_argument('-k', '--kernel', help='Kernel file to use for installation')
-    parser.add_argument('-r', '--initrd', help='InitRD file to use for installation')
-    parser.add_argument('-p', '--preseed', help='Preseed file to use for installation')
+    parser.add_argument('-i', '--image', type=url_argument, help='Image/ISO file to use for installation')
+    parser.add_argument('-k', '--kernel', type=url_argument, help='Kernel file to use for installation')
+    parser.add_argument('-r', '--initrd', type=url_argument, help='InitRD file to use for installation')
+    parser.add_argument('-p', '--preseed', type=url_argument, help='Preseed file to use for installation')
     parser.add_argument('-b', '--boot', help='Boot arguments for initial installation')
-    parser.add_argument('-x', '--xml', help='XML VM definition file')
+    parser.add_argument('-x', '--xml', type=url_argument, help='XML VM definition file')
     parser.add_argument('-g', '--gigabytes', action='append', help='Size in gigabytes of virtual disk, specify more than once for multiple disks')
     parser.add_argument('-s', '--series', choices=('hardy', 'lucid', 'natty', 'oneiric', 'precise', 'quantal'), help='Series to use for installation')
     parser.add_argument('-t', '--type', choices=('desktop', 'server', 'mini', 'alternate'), help='Install type to use for installation')
@@ -49,27 +49,6 @@
         sys.stderr.write("Please check the roadmap, as it will be included in a future version.\n")
         sys.exit(4)
 
-    url_checker = URLChecker()
-    if args.image is not None:
-        try:
-            url_checker.open(args.image)
-        except URLNotFound:
-            sys.stderr.write("ISO image not found: " + args.image + "\n")
-            sys.exit(5)
-        except URLNotReadable:
-            sys.stderr.write("ISO image not readable: " + args.image + "\n")
-            sys.exit(5)
-
-    for runlist in args.runlists:
-        try:
-            url_checker.open(runlist)
-        except URLNotFound:
-            sys.stderr.write("Run list not found: " + runlist + "\n")
-            sys.exit(5)
-        except URLNotReadable:
-            sys.stderr.write("Run list not readable: " + runlist + "\n")
-            sys.exit(5)
-
     function = run_test_vm
 
     for option in [args.boot, args.emulator, args.image, args.initrd, args.kernel, args.preseed, args.xml]:

=== modified file 'utah/provisioning/provisioning.py'
--- utah/provisioning/provisioning.py	2012-07-16 21:50:15 +0000
+++ utah/provisioning/provisioning.py	2012-07-17 14:31:55 +0000
@@ -15,6 +15,7 @@
 
 from utah.provisioning.exceptions import *
 from utah import config, iso
+from utah import UTAHException, URLNotFound, URLNotReadable
 
 class Machine(object):
     """
@@ -73,18 +74,24 @@
         self._loggersetup()
 
         for item in ['preseed', 'xml', 'kernel', 'initrd', 'image']:
-            if locals()[item] is None:
-                self.__dict__[item] = None
+            arg = locals()[item]
+            if arg is None:
+                setattr(self, item, None)
             else:
-                if '~' in locals()[item]:
-                    path = os.path.expanduser(locals()[item])
-                    self.logger.debug('Rewriting ~-based path: ' + locals()[item] + ' to: ' + path)
+                if arg.startswith('~'):
+                    path = os.path.expanduser(arg)
+                    self.logger.debug('Rewriting ~-based path: ' + arg
+                                      + ' to: ' + path)
                 else:
-                    path = locals()[item]
+                    path = arg
+
                 self.percent = 0
                 self.logger.info('Preparing ' + item + ': ' + path)
-                self.__dict__[item] = urllib.urlretrieve(path, reporthook=self.dldisplay)[0]
-                self.logger.debug(path + ' is locally available as ' + self.__dict__[item])
+                filename = urllib.urlretrieve(path,
+                                              reporthook=self.dldisplay)[0]
+                setattr(self, item, filename)
+                self.logger.debug(path + ' is locally available as '
+                                  + getattr(self, item))
 
         self.logger.debug('Machine init finished')
 

=== modified file 'utah/url.py'
--- utah/url.py	2012-07-13 10:12:53 +0000
+++ utah/url.py	2012-07-17 14:31:55 +0000
@@ -4,6 +4,7 @@
 import os
 import urllib
 import urllib2
+from argparse import ArgumentTypeError
 
 from utah.exceptions import URLNotFound, URLNotReadable
 
@@ -57,3 +58,18 @@
             raise URLNotReadable(path)
 
         return True
+
+
+def url_argument(url):
+    """
+    Check URL argument in argparse.ArgumentParser
+    """
+    url_checker = URLChecker()
+    try:
+        url_checker.open(url)
+    except URLNotFound:
+        raise ArgumentTypeError('URL not found: {0}'.format(url))
+    except URLNotReadable:
+        raise ArgumentTypeError('URL not readable: {0}'.format(url))
+
+    return url