actions should raise exceptions

actions/add_config

@@ -71,16 +71,18 @@ def main(attributes):
     url = attributes.get('url')
 
     if not url:
-        return 'Missing attribute(\'url\')'
+        raise Exception('Missing attribute(\'url\')')
 
     substitution_mode = attributes.get('substitution_mode', 'loose')
     if substitution_mode not in ['loose', 'strict']:
-        return 'Invalid option specified for substitution_mode attribute'
+        raise Exception('Invalid option specified for substitution_mode '
+                        'attribute')
 
     try:
         node.retrieve_url(url, TEMP_CONFIG)
     except Exception as exc:
-        return 'Unable to retrieve config from URL (%s)' % exc
+        raise Exception('Unable to retrieve config from URL (%s)' % 
+                        exc)
 
     contents = open(TEMP_CONFIG, 'r').read()
 
@@ -90,14 +92,16 @@ def main(attributes):
 
     if not isinstance(variables, dict):
         node.log_msg('Variables: %s' % variables)
-        return 'Unable to perform variable substitution - invalid variables'
+        raise Exception('Unable to perform variable substitution - '
+                        'invalid variables')
     try:
         if substitution_mode == 'strict':
             contents = Template(contents).substitute(variables)
         else:
             contents = Template(contents).safe_substitute(variables)
     except KeyError:
-        return 'Unable to perform variable substitution - missing variable'
+        raise Exception('Unable to perform variable substitution - '
+                        'missing variable')
 
     node.append_startup_config_lines(contents.split('\n'))
     os.remove(TEMP_CONFIG)

actions/copy_file

@@ -97,11 +97,11 @@ def main(attributes):
     src_url = attributes.get('src_url')
 
     if not src_url:
-        return 'Missing attribute(\'src_url\')'
+        raise Exception('Missing attribute(\'src_url\')')
 
     dst_url = attributes.get('dst_url')
     if not dst_url:
-        return 'Missing attribute(\'dst_url\')'
+        raise Exception('Missing attribute(\'dst_url\')')
 
     name = os.path.basename(src_url)
 
@@ -128,7 +128,7 @@ def main(attributes):
         elif overwrite == 'replace':
             pass
         else:
-            return 'Erroneous \'overwrite\' value'
+            raise Exception('Erroneous \'overwrite\' value')
 
         try:
             os.makedirs(dst_url)
@@ -143,7 +143,8 @@ def main(attributes):
             if mode is not None:
                 os.chmod(dst_path, int(str(mode), 8))
         except Exception as exc:
-            return 'Unable to retrieve file from URL (%s)' % exc
+            raise Exception('Unable to retrieve file from URL (%s)' % 
+                            exc)
     else:
         dst_path = os.path.join(PERSISTENT_DIR, name)
 
@@ -160,7 +161,7 @@ def main(attributes):
         elif overwrite == 'replace':
             lines = lines + ['sudo cp %s %s'  % (dst_path, dst_url)]
         else:
-            return 'Erroneous \'overwrite\' value'
+            raise Exception('Erroneous \'overwrite\' value')
 
         if mode:
             lines = lines + ['sudo chmod %s %s'  % (mode, dst_url)]
@@ -177,7 +178,8 @@ def main(attributes):
             node.log_msg('copy_file: saving %s '
                          'to %s' % (src_url, file_path))
         except Exception as exc:
-            return 'Unable to retrieve file from URL (%s)' % exc
+            raise Exception('Unable to retrieve file from URL (%s)' % 
+                            exc)
 
         node.log_msg('copy_file: adding rc.eos lines: \n%s$' %
                      '\n'.join(lines))

actions/install_cli_plugin

@@ -63,7 +63,7 @@ def main(attributes):
     url = attributes.get('url')
 
     if not url:
-        return 'Missing attribute(\'url\')'
+        raise Exception('Missing attribute(\'url\')')
 
     try:
         os.makedirs(PERSISTENT_PLUGIN_DIR)
@@ -76,7 +76,8 @@ def main(attributes):
         node.retrieve_url(url, '%s/%s' %
                           (PERSISTENT_PLUGIN_DIR, name))
     except Exception as exc:
-        return 'Unable to retrieve CliPlugin from URL (%s)' % exc
+        raise Exception('Unable to retrieve CliPlugin from URL (%s)' % 
+                        exc)
 
     lines = ['sudo cp %s/%s %s'  % (PERSISTENT_PLUGIN_DIR,
                                     name,

actions/install_extension

@@ -66,7 +66,7 @@ def main(attributes):
     url = attributes.get('url')
 
     if not url:
-        return 'Missing attribute(\'url\')'
+        raise Exception('Missing attribute(\'url\')')
 
     force = ast.literal_eval(
         str(attributes.get('force')))
@@ -81,7 +81,8 @@ def main(attributes):
     try:
         node.retrieve_url(url, '%s/%s' % (EXTENSIONS_DIR, name))
     except Exception as exc:
-        return 'Unable to retrieve extension from URL (%s)' % exc
+        raise Exception('Unable to retrieve extension from URL (%s)' % 
+                        exc)
 
     line = name
     if force:

actions/install_image

@@ -69,11 +69,11 @@ def main(attributes):
     url = attributes.get('url')
 
     if not url:
-        return 'Missing attribute(\'url\')'
+        raise Exception('Missing attribute(\'url\')')
 
     version = attributes.get('version')
     if not version:
-        return 'Missing attribute(\'version\')'
+        raise Exception('Missing attribute(\'version\')')
 
     current_version = node.api_enable_cmds(['show version'])[0]['version']
     if current_version == version:
@@ -83,6 +83,7 @@ def main(attributes):
     try:
         node.retrieve_url(url, '%s/%s' % (node.flash(), image))
     except Exception as exc:
-        return 'Unable to retrieve image file from URL (%s)' % exc
+        raise Exception('Unable to retrieve image file from URL (%s)' % 
+                        exc)
 
     node.api_enable_cmds(['install source flash:%s' % image])

actions/replace_config

@@ -47,10 +47,10 @@ def main(attributes):
     url = attributes.get('url')
 
     if not url:
-        return 'Missing attribute(\'url\')'
+        raise Exception('Missing attribute(\'url\')')
 
     try:
         node.retrieve_url(url, node.startup_config())
     except Exception as exc:
-        return 'Unable to retrieve config from URL (%s)' % exc.message
-
+        raise Exception('Unable to retrieve config from URL (%s)' % 
+                        exc.message)

actions/send_email

@@ -109,15 +109,15 @@ def main(attributes):
 
     smarthost = attributes.get('smarthost')
     if not smarthost:
-        return 'Missing attribute(\'smarthost\')'
+        raise Exception('Missing attribute(\'smarthost\')')
 
     sender = attributes.get('sender')
     if not sender:
-        return 'Missing attribute(\'sender\')'
+        raise Exception('Missing attribute(\'sender\')')
 
     receivers = attributes.get('receivers')
     if not receivers:
-        return 'Missing attribute(\'receivers\')'
+        raise Exception('Missing attribute(\'receivers\')')
 
     msg = MIMEMultipart.MIMEMultipart()
     msg['From'] = sender

client/bootstrap

@@ -1112,9 +1112,10 @@ def execute_action(server, action_details, special_attr):
         local_attr = action_details['attributes'] \
                      if 'attributes' in action_details \
                      else []
-        ret = module.main(Attributes(local_attr, special_attr))
-        if ret:
-            raise ZtpActionError('%s' % ret)
+        try:
+            module.main(Attributes(local_attr, special_attr))
+        except Exception as exc:
+            raise ZtpActionError(exc)
         log('Action executed succesfully (%s)' % action)
         if 'onsuccess' in action_details:
             log('Action %s: %s' % (action, action_details['onsuccess']),
@@ -1172,7 +1173,6 @@ def main():
     definition_name = definition.get('name', '')
     log('Applying definition %s' % definition_name)
 
-
     special_attr = {}
     special_attr['NODE'] = node
     for details in definition['actions']:

test/client/client_test_lib.py

@@ -219,7 +219,7 @@ def fail_action():
     return '''#!/usr/bin/env python
 
 def main(attributes):
-   return 2
+   raise Exception('Ops! I failed! :(')
 '''
 
 def erroneous_action():