#1768 improved the flow of firewall cancel and it should be able to do multiVlan and single server firewall cancels now

Author: allmightyspiff

SoftLayer/CLI/firewall/cancel.py

@@ -12,23 +12,22 @@
 
 @click.command(cls=SoftLayer.CLI.command.SLCommand, )
 @click.argument('identifier')
+@click.option('--firewall-type', required=True, show_default=True, default='vlan',
+              type=click.Choice(['vlan', 'server'], case_sensitive=False),
+              help='Firewall type.')
 @environment.pass_env
-def cli(env, identifier):
+def cli(env, identifier, firewall_type):
     """Cancels a firewall."""
 
     mgr = SoftLayer.FirewallManager(env.client)
-    firewall_type, firewall_id = firewall.parse_id(identifier)
 
     if not (env.skip_confirmations or
-            formatting.confirm("This action will cancel a firewall from your "
-                               "account. Continue?")):
+            formatting.confirm("This action will cancel a firewall from your account. Continue?")):
         raise exceptions.CLIAbort('Aborted.')
 
-    if firewall_type in ['vs', 'server']:
-        mgr.cancel_firewall(firewall_id, dedicated=False)
-    elif firewall_type == 'vlan':
-        mgr.cancel_firewall(firewall_id, dedicated=True)
+    if firewall_type == 'server':
+        mgr.cancel_firewall(identifier, dedicated=False)
     else:
-        raise exceptions.CLIAbort('Unknown firewall type: %s' % firewall_type)
+        mgr.cancel_firewall(identifier, dedicated=True)
 
     env.fout('Firewall with id %s is being cancelled!' % identifier)

SoftLayer/managers/firewall.py

@@ -157,11 +157,9 @@ def _get_fwl_billing_item(self, firewall_id, dedicated=False):
             firewall_service = self.client['Network_Component_Firewall']
         firewall = firewall_service.getObject(id=firewall_id, mask=mask)
         if firewall is None:
-            raise exceptions.SoftLayerError(
-                "Unable to find firewall %d" % firewall_id)
+            raise exceptions.SoftLayerError(f"Unable to find firewall {firewall_id}")
         if firewall.get('billingItem') is None:
-            raise exceptions.SoftLayerError(
-                "Unable to find billing item for firewall %d" % firewall_id)
+            raise exceptions.SoftLayerError(f"Unable to find billing item for firewall {firewall_id}")
 
         return firewall['billingItem']