#1345 refactored create-options a bit to have less code duplication

Author: allmightyspiff

SoftLayer/CLI/hardware/create_options.py

@@ -30,148 +30,110 @@ def cli(env, prices, location=None):
         dc_table.add_row([location_info['name'], location_info['key']])
     tables.append(dc_table)
 
-    if prices:
-        _preset_prices_table(options['sizes'], tables)
-        _os_prices_table(options['operating_systems'], tables)
-        _port_speed_prices_table(options['port_speeds'], tables)
-        _extras_prices_table(options['extras'], tables)
-    else:
-        # Presets
-        preset_table = formatting.Table(['Size', 'Value'], title="Sizes")
-        preset_table.sortby = 'Value'
-        preset_table.align = 'l'
-        for size in options['sizes']:
-            preset_table.add_row([size['name'], size['key']])
-        tables.append(preset_table)
-
-        # Operating systems
-        os_table = formatting.Table(['OS', 'Key', 'Reference Code'], title="Operating Systems")
-        os_table.sortby = 'Key'
-        os_table.align = 'l'
-        for operating_system in options['operating_systems']:
-            os_table.add_row([operating_system['name'], operating_system['key'], operating_system['referenceCode']])
-        tables.append(os_table)
-
-        # Port speed
-        port_speed_table = formatting.Table(['Network', 'Speed', 'Key'], title="Network Options")
-        port_speed_table.sortby = 'Speed'
-        port_speed_table.align = 'l'
-        for speed in options['port_speeds']:
-            port_speed_table.add_row([speed['name'], speed['speed'], speed['key']])
-        tables.append(port_speed_table)
-
-        # Extras
-        extras_table = formatting.Table(['Extra Option', 'Value'], title="Extras")
-        extras_table.sortby = 'Value'
-        extras_table.align = 'l'
-        for extra in options['extras']:
-            extras_table.add_row([extra['name'], extra['key']])
-        tables.append(extras_table)
+    tables.append(_preset_prices_table(options['sizes'], prices))
+    tables.append(_os_prices_table(options['operating_systems'], prices))
+    tables.append(_port_speed_prices_table(options['port_speeds'], prices))
+    tables.append(_extras_prices_table(options['extras'], prices))
 
+    # since this is multiple tables, this is required for a valid JSON object to be rendered.
     env.fout(formatting.listing(tables, separator='\n'))
 
 
-def _preset_prices_table(sizes, tables):
+def _preset_prices_table(sizes, prices=False):
     """Shows Server Preset options prices.
 
     :param [] sizes: List of Hardware Server sizes.
-    :param tables: Table formatting.
+    :param prices: Create a price table or not
     """
-    preset_prices_table = formatting.Table(['Size', 'Value', 'Hourly', 'Monthly'], title="Sizes Prices")
-    preset_prices_table.sortby = 'Value'
-    preset_prices_table.align = 'l'
-    for size in sizes:
-        if (_verify_prices("%.4f" % size['hourlyRecurringFee'])) or (_verify_prices("%.4f" % size['recurringFee'])):
-            preset_prices_table.add_row([size['name'], size['key'], "%.4f" % size['hourlyRecurringFee'],
-                                         "%.4f" % size['recurringFee']])
-    tables.append(preset_prices_table)
+    if prices:
+        table = formatting.Table(['Size', 'Value', 'Hourly', 'Monthly'], title="Sizes")
+        for size in sizes:
+            if size.get('hourlyRecurringFee', 0) + size.get('recurringFee', 0) + 1 > 0:
+                table.add_row([size['name'], size['key'], "%.4f" % size['hourlyRecurringFee'],
+                              "%.4f" % size['recurringFee']])
+    else:
+        table = formatting.Table(['Size', 'Value'], title="Sizes")
+        for size in sizes:
+            table.add_row([size['name'], size['key']])
+    table.sortby = 'Value'
+    table.align = 'l'
+    return table
 
 
-def _os_prices_table(operating_systems, tables):
+def _os_prices_table(operating_systems, prices=False):
     """Shows Server Operating Systems prices cost and capacity restriction.
 
     :param [] operating_systems: List of Hardware Server operating systems.
-    :param tables: Table formatting.
+    :param prices: Create a price table or not
     """
-    os_prices_table = formatting.Table(['OS Key', 'Hourly', 'Monthly', 'Restriction'],
-                                       title="Operating Systems Prices")
-    os_prices_table.sortby = 'OS Key'
-    os_prices_table.align = 'l'
-    for operating_system in operating_systems:
-        for price in operating_system['prices']:
-            if (_verify_prices(_get_price_data(price, 'hourlyRecurringFee'))) or (
-                    _verify_prices(_get_price_data(price, 'recurringFee'))):
+    if prices:
+        table = formatting.Table(['Key', 'Hourly', 'Monthly', 'Restriction'],
+                                 title="Operating Systems")
+        for operating_system in operating_systems:
+            for price in operating_system['prices']:
                 cr_max = _get_price_data(price, 'capacityRestrictionMaximum')
                 cr_min = _get_price_data(price, 'capacityRestrictionMinimum')
                 cr_type = _get_price_data(price, 'capacityRestrictionType')
-                os_prices_table.add_row(
+                table.add_row(
                     [operating_system['key'],
                      _get_price_data(price, 'hourlyRecurringFee'),
                      _get_price_data(price, 'recurringFee'),
                      "%s - %s %s" % (cr_min, cr_max, cr_type)])
-    tables.append(os_prices_table)
+    else:
+        table = formatting.Table(['OS', 'Key', 'Reference Code'], title="Operating Systems")
+        for operating_system in operating_systems:
+            table.add_row([operating_system['name'], operating_system['key'], operating_system['referenceCode']])
+
+    table.sortby = 'Key'
+    table.align = 'l'
+    return table
 
 
-def _port_speed_prices_table(port_speeds, tables):
+def _port_speed_prices_table(port_speeds, prices=False):
     """Shows Server Port Speeds prices cost and capacity restriction.
 
     :param [] port_speeds: List of Hardware Server Port Speeds.
-    :param tables: Table formatting.
+    :param prices: Create a price table or not
     """
-    port_speed_prices_table = formatting.Table(['Key', 'Speed', 'Hourly', 'Monthly', 'Restriction'],
-                                               title="Network Options Prices")
-    port_speed_prices_table.sortby = 'Speed'
-    port_speed_prices_table.align = 'l'
-    for speed in port_speeds:
-        for price in speed['prices']:
-            if (_verify_prices(_get_price_data(price, 'hourlyRecurringFee'))) or (
-                    _verify_prices(_get_price_data(price, 'recurringFee'))):
-                cr_max = _get_price_data(price, 'capacityRestrictionMaximum')
-                cr_min = _get_price_data(price, 'capacityRestrictionMinimum')
-                cr_type = _get_price_data(price, 'capacityRestrictionType')
-                port_speed_prices_table.add_row(
+    if prices:
+        table = formatting.Table(['Key', 'Speed', 'Hourly', 'Monthly'], title="Network Options")
+        for speed in port_speeds:
+            for price in speed['prices']:
+                table.add_row(
                     [speed['key'], speed['speed'],
                      _get_price_data(price, 'hourlyRecurringFee'),
-                     _get_price_data(price, 'recurringFee'),
-                     "%s - %s %s" % (cr_min, cr_max, cr_type)])
-    tables.append(port_speed_prices_table)
-
-
-def _verify_prices(prices):
-    """Verify the prices is higher to zero(0) or is '-'.
-
-    param prices:  value to verify.
-    Returns: true false.
-
-    """
-    if prices == '-':
-        return True
+                     _get_price_data(price, 'recurringFee')])
     else:
-        return float(prices) > 0
+        table = formatting.Table(['Network', 'Speed', 'Key'], title="Network Options")
+        for speed in port_speeds:
+            table.add_row([speed['name'], speed['speed'], speed['key']])
+    table.sortby = 'Speed'
+    table.align = 'l'
+    return table
 
 
-def _extras_prices_table(extras, tables):
+def _extras_prices_table(extras, prices=False):
     """Shows Server extras prices cost and capacity restriction.
 
     :param [] extras: List of Hardware Server Extras.
-    :param tables: Table formatting.
+    :param prices: Create a price table or not
     """
-    extras_prices_table = formatting.Table(['Extra Option Key', 'Hourly', 'Monthly', 'Restriction'],
-                                           title="Extras Prices")
-    extras_prices_table.align = 'l'
-    for extra in extras:
-        for price in extra['prices']:
-            if (_verify_prices(_get_price_data(price, 'hourlyRecurringFee'))) or (
-                    _verify_prices(_get_price_data(price, 'recurringFee'))):
-                cr_max = _get_price_data(price, 'capacityRestrictionMaximum')
-                cr_min = _get_price_data(price, 'capacityRestrictionMinimum')
-                cr_type = _get_price_data(price, 'capacityRestrictionType')
-                extras_prices_table.add_row(
+    if prices:
+        table = formatting.Table(['Key', 'Hourly', 'Monthly'], title="Extras")
+
+        for extra in extras:
+            for price in extra['prices']:
+                table.add_row(
                     [extra['key'],
                      _get_price_data(price, 'hourlyRecurringFee'),
-                     _get_price_data(price, 'recurringFee'),
-                     "%s - %s %s" % (cr_min, cr_max, cr_type)])
-    tables.append(extras_prices_table)
+                     _get_price_data(price, 'recurringFee')])
+    else:
+        table = formatting.Table(['Extra Option', 'Key'], title="Extras")
+        for extra in extras:
+            table.add_row([extra['name'], extra['key']])
+    table.sortby = 'Key'
+    table.align = 'l'
+    return table
 
 
 def _get_price_data(price, item):
@@ -184,26 +146,3 @@ def _get_price_data(price, item):
     if item in price:
         result = price[item]
     return result
-
-
-def _location_item_prices(location_prices, tables):
-    """Get a specific data from HS price.
-
-    :param price: Hardware Server price.
-    :param string item: Hardware Server price data.
-    """
-    location_prices_table = formatting.Table(['keyName', 'priceId', 'Hourly', 'Monthly', 'Restriction'])
-    location_prices_table.sortby = 'keyName'
-    location_prices_table.align = 'l'
-    for price in location_prices:
-        if (_verify_prices(_get_price_data(price, 'hourlyRecurringFee'))) or (
-                _verify_prices(_get_price_data(price, 'recurringFee'))):
-            cr_max = _get_price_data(price, 'capacityRestrictionMaximum')
-            cr_min = _get_price_data(price, 'capacityRestrictionMinimum')
-            cr_type = _get_price_data(price, 'capacityRestrictionType')
-            location_prices_table.add_row(
-                [price['item']['keyName'], price['id'],
-                 _get_price_data(price, 'hourlyRecurringFee'),
-                 _get_price_data(price, 'recurringFee'),
-                 "%s - %s %s" % (cr_min, cr_max, cr_type)])
-    tables.append(location_prices_table)

tests/CLI/modules/server_tests.py

@@ -368,16 +368,15 @@ def test_create_options_prices(self):
         self.assert_no_fail(result)
         output = json.loads(result.output)
         self.assertEqual(output[2][0]['Monthly'], str(0.1))
-        self.assertEqual(output[2][0]['OS Key'], 'OS_UBUNTU_14_04_LTS_TRUSTY_TAHR_64_BIT')
+        self.assertEqual(output[2][0]['Key'], 'OS_UBUNTU_14_04_LTS_TRUSTY_TAHR_64_BIT')
 
     def test_create_options_location(self):
         result = self.run_command(['server', 'create-options', '--prices', 'dal13'])
 
         self.assert_no_fail(result)
         output = json.loads(result.output)
-        print(output)
         self.assertEqual(output[2][0]['Monthly'], str(0.1))
-        self.assertEqual(output[2][0]['OS Key'], 'OS_UBUNTU_14_04_LTS_TRUSTY_TAHR_64_BIT')
+        self.assertEqual(output[2][0]['Key'], 'OS_UBUNTU_14_04_LTS_TRUSTY_TAHR_64_BIT')
 
     @mock.patch('SoftLayer.HardwareManager.place_order')
     def test_create_server(self, order_mock):