Pythonic changes (#51)

* Update constants.py

Constants should not have operations within it

* Update minion_manager.py

* Update asg_mm.py

-Constants should not have unnecessary operators
-.get() returns None by default if value is missing
-Other optimizations

* Update aws_bid_advisor.py

array itself over len()

cloud_provider/aws/asg_mm.py

@@ -1,12 +1,10 @@
 """ Metadata for each Autoscaling group in AWS. """
 MINION_MANAGER_LABEL = 'k8s-minion-manager'
-NOT_TERMINATE_LABEL = MINION_MANAGER_LABEL+'/not-terminate'
+NOT_TERMINATE_LABEL = 'k8s-minion-manager/not-terminate'
 
 
 class AWSAutoscalinGroupMM(object):
-    """
-    This class has metadata associated with an autoscaling group.
-    """
+    """This class has metadata associated with an autoscaling group."""
 
     def __init__(self):
         # 'asg_info' is populated with the returned value of
@@ -32,11 +30,11 @@ def set_asg_info(self, asg_info):
         assert asg_info is not None, "Can't set ASG info to None!"
         self.asg_info = asg_info
         for tag in self.asg_info['Tags']:
-            if tag.get('Key', None) == MINION_MANAGER_LABEL:
+            if tag.get('Key') == MINION_MANAGER_LABEL:
                 if tag['Value'] not in ("use-spot", "no-spot"):
                     tag['Value'] = 'no-spot'
-            elif tag.get('Key', None) == NOT_TERMINATE_LABEL:
-                if tag['Value'] in ('true', 'True', 'TRUE'):
+            elif tag.get('Key') == NOT_TERMINATE_LABEL:
+                if tag['Value'].lower() == 'true':
                     tag['Value'] = True
                 else:
                     tag['Value'] = False
@@ -64,20 +62,15 @@ def get_bid_info(self):
         return self.bid_info
 
     def add_instances(self, instances):
-        """
-        Adds the given instances to the 'instances' array if they're not
+        """Adds the given instances to the 'instances' array if they're not
         present.
         """
         for instance in instances:
-            if instance.InstanceId in self.instance_info.keys():
-                continue
-            else:
+            if instance.InstanceId not in self.instance_info:
                 self.instance_info[instance.InstanceId] = instance
 
     def remove_instance(self, instance_id):
-        """
-        Removes the given instance from the 'instances' array.
-        """
+        """Removes the given instance from the 'instances' array."""
         self.instance_info.pop(instance_id, None)
 
     def get_instance_info(self):
@@ -90,7 +83,7 @@ def get_instances(self):
 
     def get_mm_tag(self):
         for tag in self.asg_info['Tags']:
-            if tag.get('Key', None) == MINION_MANAGER_LABEL:
+            if tag.get('Key') == MINION_MANAGER_LABEL:
                 return tag['Value'].lower()
         return "no-spot"
 
@@ -119,6 +112,6 @@ def is_instance_running(self, instance):
     def not_terminate_instance(self):
         """ Retures is the ASG set not terminate instance """
         for tag in self.asg_info['Tags']:
-            if tag.get('Key', None) == NOT_TERMINATE_LABEL:
+            if tag.get('Key') == NOT_TERMINATE_LABEL:
                 return tag['Value']
         return False

cloud_provider/aws/aws_bid_advisor.py

@@ -209,7 +209,7 @@ def run(self):
 
     def run(self):
         """ Main method of the AWSBidAdvisor. """
-        if len(self.all_bid_advisor_threads) > 0:
+        if self.all_bid_advisor_threads:
             logger.debug("BidAdvisor already running!")
             return
 
@@ -234,8 +234,7 @@ def run(self):
             logger.info("Waiting for initial pricing information...")
             try:
                 with self.lock:
-                    if len(self.on_demand_price_dict) != 0 and \
-                            len(self.spot_price_list) != 0:
+                    if self.on_demand_price_dict and self.spot_price_list:
                         return
             finally:
                 time.sleep(SECONDS_PER_MINUTE)
@@ -326,8 +325,7 @@ def get_new_bid(self, zones, instance_type):
         """
 
         with self.lock:
-            if len(self.on_demand_price_dict) == 0 or \
-                    len(self.spot_price_list) == 0:
+            if not self.on_demand_price_dict or not self.spot_price_list:
                 logger.info("Pricing data not available! Using DEFAULT_BID")
                 return DEFAULT_BID
 

constants.py

@@ -1,4 +1,4 @@
 """Constants used by the minion-manager."""
 
 SECONDS_PER_MINUTE = 60
-SECONDS_PER_HOUR = (60 * 60)
+SECONDS_PER_HOUR = 3600

minion_manager.py

@@ -20,7 +20,7 @@ def validate_usr_args(usr_args):
     Validates the arguments provided by the user.
     """
     assert usr_args.cloud.lower() == "aws", "Only AWS is currently supported."
-    assert usr_args.cluster_name != None, "Cluster name is required"
+    assert usr_args.cluster_name is not None, "Cluster name is required"
     if "profile" not in usr_args:
         usr_args.profile = None
 
@@ -39,7 +39,8 @@ def run():
     parser.add_argument("--profile", help="Credentials profile to use")
     parser.add_argument("--refresh-interval-seconds", default="300",
                         help="Interval in seconds at which to query AWS")
-    parser.add_argument("--cluster-name", required=True, help="Name of the Kubernetes cluster. Get's used for identifying ASGs")
+    parser.add_argument("--cluster-name", required=True,
+                        help="Name of the Kubernetes cluster. Get's used for identifying ASGs")
 
     usr_args = parser.parse_args()
     validate_usr_args(usr_args)